Open clakech opened 4 years ago
clakech
commented
4 years ago Ok. GitHub expect a response I less than 10 seconds https://developer.github.com/v3/guides/best-practices-for-integrators/
In our case, policy-bot is too slow here.
Any clue to avoid this? Async config to run hook later would help.
bluekeyes
commented
4 years ago Thanks for reporting this. I'm not sure about the 10 second timeout (which may be a different issue), but the initial error looks very similar to an issue on our Bulldozer application: https://github.com/palantir/bulldozer/issues/149.
In that case, I found and fixed a potential race condition (https://github.com/palantir/go-githubapp/pull/24) in the shared library used by both of these apps, but it seems like that was not the problem, and I haven't had a chance to debug further. Building a test binary with Go's race detector is probably the next step.
clakech
commented
4 years ago This 10 seconds requierement is documented in the github documentation:https://developer.github.com/v3/guides/best-practices-for-integrators/
GitHub expects that integrations respond within 10 seconds of receiving the webhook payload. If your service takes longer than that to complete, then GitHub terminates the connection and the payload is lost.Thank you for the pointer to this bulldozer issue. (we are using bulldozer too) Maybe we have a cache issue here and we should not call github APIs so many times OR maybe we need to invalidate the cache when we change some github teams/config. Let's investigate deeper on this issue.
clakech
commented
4 years ago BTW, Github recommandation suggest to use another software architecture for this kind of bot:
Favor asynchronous work over synchronous
...
Since it's impossible to predict how fast your service will complete, you should do all of "the real work" in a background job. Resque (for Ruby), RQ (for Python), or RabbitMQ (for Java) are examples of libraries that can handle queuing and processing of background jobs.
Note that even with a background job running, GitHub still expects your server to respond within ten seconds. Your server needs to acknowledge that it received the payload by sending some sort of response. It's critical that your service performs any validations on a payload as soon as possible, so that you can accurately report whether your server will continue with the request or not.We are confident with this timeout issue because we have a HTTP 499 error in our load-balancer (traefik) and that means we have a "Client Closed Request" from the Github side.
bluekeyes
commented
4 years ago I believe the originally reported panic is fixed in 1.12.4; unfortunately, the panic occurred when there was a failure communicating with GitHub, so requests that previously panicked will still probably fail.
Any issues due to processing times higher than GitHub's timeout are still present and will probably require a bigger architecture change to solve.
Hi,
Policy bot is working fine since more than 6 months and we face an issue when we create pull requests (~200/250 files) from a release branch to master branch: "panic: runtime error: invalid memory address or nil pointer dereference"
The stack trace is really strange and we do not understand what is going on. We suspect an issue with the githubapp hook that send logs/metrics when we try to get datas from github membership config.
For the same hook message, sometimes it works fine, sometime it fails. (we can retry previous event hook form github UI and check roundtrip result)
Here is one stacktrace:
We use latest version 1.12.3 of policy-bot deployed with docker/k8s on GCP with a traefik in between.
On fail we can found an error in github UI, in the policy bot app config advanced recent deliveries: "We couldn’t deliver this payload: Service Timeout"
Any clue would help here.
Thank you for