cli: avoid os.Exit() in Run() funcs#7788
Conversation
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
srenatus
force-pushed
the
sr/pnuszowyrsnp
branch
3 times, most recently
from
8e8bfbf to
679ca32
Compare
…f OPA `os.Exit` immediately exits the program and doesn't run defer functions. This can be problematic as any command.OnFinalize routines and any logic after the command.Execute won't be run. Also suppress all RunE cobra error and usage messages. These would be printed twice otherwise. Signed-off-by: Stephan Renatus <stephan@styra.com> Co-authored-by: Kevin St. Pierre <kevin@styra.com>
Signed-off-by: Stephan Renatus <stephan@styra.com>
srenatus
force-pushed
the
sr/pnuszowyrsnp
branch
from
679ca32 to
b369ea7
Compare
| os.Exit(1) | ||
| var e *cmd.ExitError | ||
| if errors.As(err, &e) { | ||
| exit = e.Exit |
There was a problem hiding this comment.
Any wrapped error will be unwrapped and reported by cobra?
There was a problem hiding this comment.
Or that is what cmd.SilenceErrors = true turns off? When do we benefit from wrapping an error?
There was a problem hiding this comment.
Not just yet -- but that's follow-up stuff. 🤞
There was a problem hiding this comment.
startWatcher() still has an os.Exit(1), but it's run in a goroutine, so maybe it's tricky to pull out of there.
There was a problem hiding this comment.
There is still an os.Exit(1) in init(). Dropping it wouldn't impact what you're doing here, from what I understand, but it raises the question: do we need that? If we fail to deprecate a flag, maybe we don't really care and should prioritize a non-bricked build(?).
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| cmd.SilenceErrors = true | ||
| cmd.SilenceUsage = true | ||
| return generateCmdOutput(os.Stdout, check) |
There was a problem hiding this comment.
Here we shouldn't wrap the error?
There was a problem hiding this comment.
Not really, it's not evaluating user-controlled rego code...
3 participants
os.Exitimmediately exits the program and doesn't run defer functions.This can be problematic as any command.OnFinalize routines and any logic
after the command.Execute won't be run.
Also suppress all RunE cobra error and usage messages. These would be
printed twice otherwise.
This also wraps any Rego errors. In some follow-up work, we'll try to enhance
error messages according to whatever happened during
opa eval. 🤞