Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upCatch error for commands with output capture = false #175
Conversation
…nse=false
|
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. |

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

ivanpesin commentedSep 16, 2017
•
edited
This is a proposed fix for inconsistency in handling exit status of command with
include-command-output-in-responseset totrueandfalse.Currently, if
include-command-output-in-response: trueand command exits with error code != 0,webhookwill return error code500. However, when set tofalse,webhookignores 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
500if 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.