Return response body on matched handler

[MLBMatt]

Thanks a lot for providing this library. I was wondering if there was anyway to return a custom response when a handler is matched? Currently it only returns "ok" however I'd like to return a custom response body if possible. Thanks!

[gr2m]

I'm afraid not at this point, the response is hardcoded:

webhooks.js/middleware/middleware.js

Line 55 in 771d51a

response.end('ok\n')

I would recommend to build your own middleware, you can use https://github.com/octokit/webhooks.js/tree/master/middleware as a reference

I'm curious though, what is your use case?

[MLBMatt]

We're leveraging Tekton (https://github.com/tektoncd/pipeline) pipelines as our CI and using Github events to trigger builds.

Currently the only way to do any kind of filtering/conditional build triggers is to create "Interceptors" (https://github.com/tektoncd/triggers/blob/master/docs/eventlisteners.md#event-interceptors) within Tekton that capture the GH hook payload and allow one to return a 200 and modified payload (to run the job, the subsequent payload is leveraged within Tekton) or a 5xx (in which the job is not ran).

The GH hook payload is essentially proxied within Tekton and manipulated into a form to use within a Tekton pipeline. Currently though, leveraging this library provides no way to return what would later be used within a Tekton pipeline because of the inability to return values back to Tekton.

[MLBMatt]

@gr2m How relevant is the use case that there are multiple handlers listening for the same action type? I ask because we could take the first result returned from a hook and return it as the response body, or we could do some kind of merging of all hook returns into the final response body. I'm forking and mocking something up now but just curious to get your's (and any other maintainer's) opinions. Thanks

[gr2m]

Yeah it's a bit complicated. There is a 10s timeout at what point the webhook delivery is canceled and GitHub considers the delivery an error. So we would need to consider that as well.

One thought I had is to expose a special log method which would add something to the webhook response. If the logs got cut off because of a timeout, it would add a "truncated": true key to the response, or something similar. That would at least make it how output is created in case of multiple handlers for the same event.

Your approach with merging returned data is interesting, too! The problem with the 10s cutoff remains and your approach is all-or-nothing, while a log method could add data to the response throughout each event handler without the possibility of conflicts

[MLBMatt]

Doesn't that 10s issue already exist as its Promise.alling the hooks?

I would assume too that if you've defined multiple hooks for a given event, you'd want the request to fail if it took longer than 10s, no? I would be weary sending some data and not all data at the risk of the downstream service assuming all data to work correctly. If one has defined an event in multiple hooks, I'd assume the intention by the developer is all that data is going to be eventually returned back. Thoughts?

[gr2m]

Doesn't that 10s issue already exist as its Promise.alling the hooks?

Yes, the problem exists in the current implementation, too.

I would assume too that if you've defined multiple hooks for a given event, you'd want the request to fail if it took longer than 10s, no?

Not necessarily. The more common request is that the webhook handler responds immediately, to show that the webhook was received successfully. If an error occurs in the handler than that's a problem that needs to be handled by the app, not reported to GitHub.

I wonder you can decouple the response to GitHub from the response you parse internally, so you don't need to worry about the 10s timeout?

I still think that in your special case, I'd write my own middleware

[MLBMatt]

Just for more context, the result of the hooks running never makes it back to Github (in my case). The proxy (Interceptor) automatically sends a pass/fail to GH when received and the Interceptor does work with the GH event payload that eventually makes it to the pipeline.

So in my specific instance, I'll never run into the 10s issue since the result of the hooks never makes it back to GH, its a response from the proxy (a simple pass/fail).

TBH, the middleware does 99% of what I need minus the return part, which I'll just maintain a fork for internally. Thanks for your help/advice @gr2m.

[gr2m]

Thanks Matt for the insights! I'll keep your use case in mind, if it turns out to be more common then I'd be open to make the middleware more flexible.

Is this the fork you plan to maintain? https://github.com/MLBMatt/webhooks.js/tree/add-response-bodies It might be helpful for others in future