-
-
Notifications
You must be signed in to change notification settings - Fork 817
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
Catch error for commands with output capture = false #175
base: development
Are you sure you want to change the base?
Conversation
Unfortunately, this approach will hang the request while the command is executing (even though we're not interested in it's output). That is the reason I used a separate goroutine in this case, so I can fire off the command in the background and close the request. :-( |
Ah ok, I haven't realized you intended this way. Then I have two questions:
Let me know your thoughts, I think I can get this done this Saturday if you agree to this. |
We actually have the opposite behavior, we have a special config parameter if you want for request to wait the script execution (
Either case would be a step forward, but maybe we could store the list of currently running hook commands and count them instead, and clean them out periodically using a separate goproc?
@ivanpesin sorry for the late reply 😂 |
@adnanh Later is better than never! :) I'll take a stab at it over the weekend. |
This is a proposed fix for inconsistency in handling exit status of command with
include-command-output-in-response
set totrue
andfalse
.Currently, if
include-command-output-in-response: true
and command exits with error code != 0,webhook
will return error code500
. However, when set tofalse
,webhook
ignores errors and always returns code200
.Moreover, there are two different paths in the code for each case, which seems as additional inconsistency.
This PR ensures command is spawned in a consistent manner and returns code
500
if command fails in both output capturing and non-capturing modes. There is no need to spawn a separate goroutine, because handler is spawned as a goroutine when the request comes in. This PR executes command in the same goroutine as the handler.It also fixes issues we have discussed in PR #167:
Let me know if this solution works.