-
-
Notifications
You must be signed in to change notification settings - Fork 870
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make webhooks more resilient #2516
Conversation
@abraunegg It is probably easier to review this with |
@Lyncredible Webhooks is still missing from v2.5 branch, due to be added back in shortly, so what I will do, once those changes are added back in, circle back and either cherry pick your changes or ask you to re-work your PR to align to v2.5 |
@abraunegg Sounds good. Take your time. My local box is already running on this PR (on top of 4a60654 in |
* Do not quit on subscription API errors. Instead, wait a configurable interval before retrying. * Shorten default webhook expiration and renewal intervals to reduce the chance of http 409 errors. * Handle http 409 on subscription creation by taking over the existing subscription. * Handle http 404 on subscription renewal by creating a new subscription(current behavior, listed for completeness). * Log other known http errors include 400 (bad webhook endpoint), 401 (auth failed), and 403 (too many subscriptions). * Log detailed messages when encoutering unknown http errors to assist with future debugging.
6fc4bbf
to
30ee83a
Compare
The 'alpha-2' PR (#2505) is just about 'feature parity complete' - with the exception of adding back in webhooks. I am going to start doing that tomorrow or the next day as part of 'alpha-3' - but it would be great once that lands if you can review and/or test to ensure that things are working correctly. |
@Lyncredible |
* Update webhook feature with #2516 changes
@Lyncredible |
@abraunegg It has been working fine for the last ~12 hours. Closing this PR. Thanks for incorporating the fix! |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fixes #2501