Webhook: [enhancement] allow raw responses

[mullender]

Some webhooks allow you to return data in the response, e.g. hipchat webhooks

If include-command-output-in-response is set to true the output is wrapped inside a json, which prevents responding with a valid json as expected by the caller of the webhook.

An enhancement would be to allow raw-command-output-in-response which would simply pass all stdout content to the http response body, without wrapping it.

[moorereason]

I have a potential fix in my repo, but I can't create a PR against the development branch because dev is behind master (merge conflicts). @adnanh, once you get the dev branch in sync, I can create a PR.

[adnanh]

What to do with stderr? Should it be merged with stdout, or just print out the stdout without stderr?

[moorereason]

I think you log stderr and only return stdout.

[adnanh]

This will be added in 2.5.0.

include-command-output-in-response flag will now make webhook return raw stdout and log stderr.

[kachkaev]

Thanks @adnanh, this change makes a real difference! I tested it just now and would like to ask you two things:

1. What do you think of including the output of failed commands just as it's done for the succeeded ones? I added exit 42 to my run.sh and started getting 500 as expected. At the same time, I stopped seeing all the nice output I expected before the last (malformed) command. Webhooks are often used in CI scripts and it's useful to see why a particular build has failed without going to the production server and scrolling through the logs.
2. I've noticed that response-message is not showing when include-command-output-in-response is set to true. The build script I have is checking the result of the webhook against the presence of a response message and considers itself successful only if it is found. I know that I can use HTTP 200/500 for the same check now when include-command-output-in-response is on, but the current behaviour is still not intuitively obvious. An extra check can be useful e.g. to protect yourself from the webhook endpoint being replaced with smth else that always returns 200. How about appending response-message on success in all cases and thus remove a not-very-intuitive side-effect of include-command-output-in-response?

Thanks again for your work on this project! It's one of those nice little things that make people's lives better :-)

[adnanh]

Hello :-)

Regarding the point number 1:
I propose adding a flag to include stdout & stderror in failed executions and leave the current behavior as default.

Regarding the point number 2:
You could wrap your command in a script that echoes your string to stdout and then runs the original command, and you have restored the old functionality. :-)

Having said that, I personally believe that the described use-case should be modified to use headers as a transport of such details.

You can define custom response headers on hook execution using response-headers setting in hooks.json, or on all hooks by using -header flag when starting webhook (you can find more details about those fields & flags on the wiki pages). i.e. you could set 'X-Served-By' header to return 'webhook' and check for it's existence in your build script, rather than mixing custom response message & the command output.

Cheers!

[gdubicki]

Hey @adnanh,

I would like to make sure: you proposal from point 1 was not implemented, right? As of now if you return exit code != 0 and use include-command-output-in-response then you lose the output?