-
Notifications
You must be signed in to change notification settings - Fork 37
go: add syntax for error handling to Neon Go #1393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
After a recent announcement [1] by Go team, saying they won't proceed with adding error handling syntax to Go, it is evident we have to implement a solution downstream. We already build Linux kernel with custom patches (see #1386, #1392), it is logical that as a next step we should have our own fork of Go, called Neon Go. The first feature is a syntax to handle errors. It supports the following syntax: ... := ... handle err { ... } Which is equivalent to: ..., err := ... if err != nil { ... } Advantages: 1. Easy to read. 2. Easy to write (no need to look for the ! key). Timeline to implement: 5 June - merge to main 6 June - deploy to staging 7 June - deploy to production [1]: https://go.dev/blog/error-syntax NOTE: this is, of course, a joke. I don't have any strong opinion about error handling in Go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new downstream syntax for simplified error handling (handle err { ... }) across the Neon Go codebase and integrates desugaring/resugaring scripts into the build pipeline.
- Replaces explicit
if err != nilblocks with the concisehandle err { ... }sugar in many packages. - Adds
desugar/resugarMakefile targets and makesbuilddepend ondesugarto apply transformations before vetting and compilation. - Updates CI‐relevant files and helper functions to use the new syntax uniformly.
Reviewed Changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/neonvm/controllers/vm_qmp_queries.go | Replaced QMP error checks with handle err blocks. |
| pkg/neonvm/controllers/vm_controller_cpu_scaling.go | Applied handle err sugar in CPU scaling logic. |
| pkg/neonvm/controllers/vm_controller.go | Sugar‐ized reconciler steps (IPAM, certs, pods). |
| pkg/neonvm/controllers/runner_cpu_limits.go | Simplified error handling in runner CPU limits. |
| pkg/neonvm/controllers/certs.go | Used handle err for cert decoding and generation. |
| pkg/api/vminfo.go | Sugar in VM info extraction functions. |
| pkg/agent/watch.go | Applied sugar in VM watcher event creation. |
| pkg/agent/scalingevents/reporter.go | Sugar in NewReporter initialization. |
| pkg/agent/scalingevents/clients.go | Used sugar in client‐creation helpers. |
| pkg/agent/runner.go | Replaced HTTP and dispatcher error checks. |
| pkg/agent/entrypoint.go | Sugar in startup (Run) error flows. |
| pkg/agent/dumpstate.go | Simplified listener and dump error handling. |
| pkg/agent/dispatcher.go | Sugar in dispatcher and websocket connector. |
| pkg/agent/core/testhelpers/clock.go | Applied sugar to clock initialization. |
| pkg/agent/core/state_test.go | Used sugar in test helper (duration parsing). |
| pkg/agent/core/metrics.go | Sugar in metrics parsing and label extraction. |
| pkg/agent/config.go | Simplified config file open error. |
| pkg/agent/billing/clients.go | Sugar in billing client setup. |
| pkg/agent/billing/billing.go | Used sugar in NewMetricsCollector. |
| Makefile | Added .PHONY targets for desugar/resugar and updated build dependency. |
Comments suppressed due to low confidence (2)
Makefile:166
- [nitpick] Add a note in the README or contributor guide explaining how and when to run
scripts/desugar.shandscripts/resugar.sh, so new developers understand the error-handling workflow.
.PHONY: resugar
Makefile:158
- Consider updating the CI configuration (e.g., GitHub Actions or other pipelines) to invoke
make desugarbefore builds, ensuring the error-sugar transformations are applied consistently in all environments.
build: desugar vet bin/vm-builder ## Build all neonvm binaries.
Should be first line of the PR 😆 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't read, but looks good to me 🚀
After a recent announcement 1 by Go team, saying they won't proceed
with adding error handling syntax to Go, it is evident we have to
implement a solution downstream.
We already build Linux kernel with custom patches (see #1386, #1392), it
is logical that as a next step we should have our own fork of Go, called
Neon Go.
The first feature is a syntax to handle errors. It supports the
following syntax:
... := ... handle err {
...
}
Which is equivalent to:
..., err := ...
if err != nil {
...
}
Advantages:
Timeline to implement:
5 June - merge to main
6 June - deploy to staging
7 June - deploy to production
Also, disable parts of the CI, as it doesn't have good support for Neon Go (yet).
NOTE: this is, of course, a joke. I don't have any strong opinions about
error handling in Go.