Don't fail on err in exec #259
Conversation
otelcli/exec.go
Outdated
| @@ -94,7 +95,10 @@ func doExec(cmd *cobra.Command, args []string) { | |||
| } | |||
|
|
|||
| if err := child.Run(); err != nil { | |||
| config.SoftFail("command failed: %s", err) | |||
| span.Status = & tracev1.Status { | |||
| Message: fmt.Sprintln("exec command failed: ", err), | |||
There was a problem hiding this comment.
I'd just do fmt.Sprintf here, message doesn't need the newline.
data_for_test.go
Outdated
| // Config: otelcli.DefaultConfig().WithEndpoint("grpc://{{endpoint}}"), | ||
| // }, | ||
| // }, | ||
| // }, |
There was a problem hiding this comment.
Hmmm this test looks right.
Try adding "--fail", "--verbose" to the CliArgs. It looks like config.SoftFail is being called when child.Run() returns an err.
If the command starts but does not complete successfully, the error is of type *ExitError. Other error types may be returned for other situations.
so the if err :- child.Run(); err != nil needs to be expanded to check for ExitError and not SoftFail on that so you can send the error upstream in the span status.
|
Unfortunately, I don't have further cycles to put towards this. Thanks for the review and the time/effort you've spent on this tool! |
|
Thanks for the PR! Work is busy right now so it might take me a couple weeks but I'll get this in and release. |
Added ExitCode to the results struct so it's easier to test against. I'm trying to remove Diagnostics eventually so it should move anyways. Reworked some failure catching to be more robust for exec. Tests pass :)
Fixes #258
I'm having a hard time writing a test for this. I have verified that this change makes the code do what I want - spans show up in Honeycomb and
otel-cli exec falsehas a non-0 exit code.However, the test does not appear to work? Locally it tells me that no spans were recorded.