[gitlab] get rid of UnhandledPromiseRejectionWarning #10148
Conversation
| res.send(); | ||
| return; | ||
| try { | ||
| const event = req.header("X-Gitlab-Event"); |
There was a problem hiding this comment.
Does Gitlab send any kind of signature with the payload that we should verify, like Github does?
There was a problem hiding this comment.
|
Thanks for the PR. Would it make sense to write a test case for this? |
@easyCZ, yes, sure. Is this a general question, do you want want to help, or do you want me to create one? |
Sorry, I should've been clearer. Normally, when you encounter a 3rd party integration behaviour which is unexpected, it's useful to add a regression test to ensure the special treatment doesn't get undone without necessary context. It also helps as documentation of the behaviour. My question was more how difficult it would be for you to add it, or whether you feel it is necessary for this case. |
|
@easyCZ I stongly agree that we should have tests for integrations, but I don't want to disturb focus now, and feel it's out-of scope for this PR. @AlexTugarev Could could you rebase + add a couple of lines of comments as to why the changes are necessary/how GitLab behaves? 🙏 |
There was a problem hiding this comment.
Fixes the issue at hand! 👍
/hold because of this comment: Please add a couple of lines about the why
AlexTugarev
force-pushed
the
at/gitlab-app
branch
from
74434b2 to
cc5560a
Compare
|
@geropl, thanks for the pointer, while updating the PR description as requested rightly, I took a change to strictly follow the recommendations on webhook receiver implementations, thus rewrote the part again to make it clean. |
| }); | ||
|
|
||
| span.finish(); | ||
| res.status(201).send("Prebuild request handled."); |
There was a problem hiding this comment.
BTW, this actually can be used to check the handling of the event on GitLab's side. quite handy!
the 201 is to signal the async handling on Gitpod's side.
| @@ -32,49 +32,55 @@ export class GitLabApp { | |||
|
|
|||
| @postConstruct() | |||
| protected init() { | |||
| /** | |||
| * see https://docs.gitlab.com/ee/user/project/integrations/webhooks.html#configure-your-webhook-receiver-endpoint | |||
|
/hold cancel |
roboquat
added
deployed: webapp
This PR fixes several issues with the GitLab webhook receiver:
UnhandledPromiseRejectionWarning