feat: Allow Kong to exit with semantic exit codes #507
Conversation
There was a problem hiding this comment.
I don't mind the idea of errors being able to override exit codes, but the idiomatic way to do that would be having the error itself implement an interface, eg.
type ExitCode interface {
ExitCode() int
}
var exitCode ExitCode
if errors.As(err, &exitCode) {
os.Exit(exitCode.ExitCode())
}This would allow Kong to opt in to error codes without pulling in an external dependency.
boblail
force-pushed
the
lail/semantic-exit-codes
branch
from
06a689b to
8b4d4cc
Compare
boblail
force-pushed
the
lail/semantic-exit-codes
branch
from
8b4d4cc to
89f99d1
Compare
|
@alecthomas, makes sense, thanks! I've updated this PR (and square/exit) accordingly. |
kong.go
Outdated
| return nil, err | ||
| } | ||
| if ctx.Error != nil { | ||
| return nil, &ParseError{error: ctx.Error, Context: ctx} | ||
| return nil, exitAsUsageError(&ParseError{error: ctx.Error, Context: ctx}) |
There was a problem hiding this comment.
Maybe ParseError could just implement ExitCode() int { return 80 }?
There was a problem hiding this comment.
At first, I wrapped all the ParseErrors but then I realized that the calls to k.applyHook invoked code outside of Kong — which could presumably fail for any reason, right?
But as I read Kong's code, it looked like the only reasons that Trace, Reset, Resolve, Apply, and Validate would fail would be malformed args or environment variables or config files it was trying to slurp in. Does that sound right?
There was a problem hiding this comment.
That's correct. Only hooks or Run() should execute code outside of Kong's control.
There was a problem hiding this comment.
Is that relevant to my comment though?
There was a problem hiding this comment.
Oh, yes! b/c Parse wraps user code in ParseError so not every ParseError is necessarily a usage error?
There was a problem hiding this comment.
Well, I think for those cases we shouldn't wrap in ParseError
There was a problem hiding this comment.
Gotcha!
I've added a commit to make that change: 2a7471f
ParseErrorimplementsExitCode()and returns 80- Errors returned from
k.applyHookare not wrapped inParseError
This makes sense to me. Do you consider it a breaking change if the error type changes?
There was a problem hiding this comment.
Let's merge this in, then I'll have a think about it.
boblail
force-pushed
the
lail/semantic-exit-codes
branch
from
89f99d1 to
dd27ef0
Compare
I made the interface public, gave it a docstring, and referenced it in FatalIfErrorf's doc string. Do you think it warrants a mention in the README in addition? |
At Block, we've instrumented a number of commandline tools and set SLOs on some tools' reliability. To do that effectively, we had to partition usage errors from reliability issues. We looked at [prior art](https://github.com/square/exit?tab=readme-ov-file#reserved-codes-and-prior-art) and, taking inspiration from HTTP, defined [a set of semantic exit codes](https://github.com/square/exit?tab=readme-ov-file#about) in ranges: 80-99 for user errors, 100-119 for system errors. We've been wrapping errors in `exit.Error` at whatever level of the stack can tell which class an error is and unwrapping them at exit (`os.Exit(exit.FromError(err))`). This adds support for semantic exit codes to Kong, to `FatalIfErrorf`, which is used internally by `kong.Parse` and often used in Kong applications.
…ap errors from hooks in `ParseError`
boblail
force-pushed
the
lail/semantic-exit-codes
branch
from
dd27ef0 to
2a7471f
Compare
This pull request has 3 commits. Kong v0.9.0 has a couple of breaking changes for `hermit`, so: 1. The first commit updates from Kong 0.5.0 → 0.8.1 safely 2. The second commit updates to Kong 0.9.0 and sorts out the changes 3. The third commit updates from Kong 0.9.0 → 1.9.0 safely - [x] (Ideally I'd merge alecthomas/kong#507 and then bump this to the next version) fixes #444
feat: Allow Kong to exit with semantic exit codes
At Block, we've instrumented a number of commandline tools and set SLOs on some tools' reliability. To do that effectively, we had to partition usage errors from reliability issues. We looked at prior art and, taking inspiration from HTTP, defined a set of semantic exit codes in ranges: 80-99 for user errors, 100-119 for system errors.
We've been wrapping errors in
exit.Errorat whatever level of the stack can tell which class an error is and unwrapping them at exit (os.Exit(exit.FromError(err))).This adds support for semantic exit codes to Kong, to
FatalIfErrorf, which is used internally bykong.Parseand often used in Kong applications.It also exits 80 (usage error) instead of 1 (generic error) when Kong parses a syntactically or structurally invalid command line.