permit empty string for ErrPrefix #2226
Comments
|
@jchappelow Hi I am thinking about how to do this and I was wondering if you would like a variable such as "HideErrPrefix" that you can set to true and it will auto set the prefix to the empty string or just simply be able to set the prefix to "" and not change anything else? |
Either works, yeah. For the latter, that's what I'm getting at here: The use of |
jchappelow
added a commit
to kwilteam/kwil-db
that referenced
this issue
Feb 3, 2025
This changes how the command line apps handle errors to ensure we have non-zero exit codes to the OS/shell while also maintaining our own output format. This is important for scripting among other things. The problem with cobra is that if we return an error from say `RunE`, [cobra will insist on prefixing the error message](spf13/cobra#2226) with a text like `Error: ` which is no good for reasons. We can sorta set this prefix, but it's a somewhat flawed implementation whereby we always end up with at least two spaces prefixing the output we want. This is why we have had the `shared/display.PrintErr` always eating the error, I believe. We get around this issue by continuing to swallow the error, but storing the error in the command's context, and then in `main()` pulling it out and setting the exit code if it is not nil. (we don't actually need the `error`, just a bool, but I include the whole thing for debugging). Finally, this PR also resolves related issues with startup and shutdown: - the `buildServer` function was receiving the background context (my bad) so we couldn't really cancel things like a peer dial that would have to timeout instead - the `P2Pservice` initializer used a background context - the bootnode loop in `(*P2PService).Start` always returned nil even if the context was cancelled and a peer connect returned a non-nil error. This was logically done to allow the loop to `continue` to other peers, but if the context was cancelled, we need to abort startup. Before this fix, it would go on to initialize other systems, eventually hitting one that cared about the context cancellation. Finally, his fixes the peer filtering integration test so that the health check is disabled in this test.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The
SetErrPrefixmethod introduced in #2023 is nice, but I wanted to set it to an empty string"", but discovered that this is treated the same as if you had not set it at all. For example, I want to print structured JSON with an"error"field or similar. To do this and return a non-zero exit code to the OS requires more hoop jumping unless I can remove teh prefix.It would be preferable if this were a
*stringfield, which would allow setting it to an empty string.Another unexpected aspect of the prefix, is that a space is always inserted because it is applied with
c.PrintErrln(c.ErrPrefix(), err.Error()). Perhaps at this spot, it could simply runc.PrintErrln(err.Error())if we find thatc.ErrPrefix()is""? This way the prefix field could remain astring(no pointer).The text was updated successfully, but these errors were encountered: