From e4bca156647f0617827a89ea0b6db132eb18e021 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 11 Dec 2023 16:50:43 +0000 Subject: [PATCH] Log a warning when being ratelimtted by Github --- .../apps/api/tests/test_github_webhook_filter.py | 16 ++++++++++++++++ pydis_site/apps/api/views.py | 9 +++++++++ 2 files changed, 25 insertions(+) diff --git a/pydis_site/apps/api/tests/test_github_webhook_filter.py b/pydis_site/apps/api/tests/test_github_webhook_filter.py index 2c9f59e52b..8ca6051199 100644 --- a/pydis_site/apps/api/tests/test_github_webhook_filter.py +++ b/pydis_site/apps/api/tests/test_github_webhook_filter.py @@ -1,8 +1,10 @@ from unittest import mock +from urllib.error import HTTPError from django.urls import reverse from rest_framework.test import APITestCase +from pydis_site.apps.api.views import GitHubWebhookFilterView class GitHubWebhookFilterAPITests(APITestCase): def test_ignores_bot_sender(self): @@ -44,3 +46,17 @@ def test_accepts_interesting_events(self): response = self.client.post(url, data=payload, headers=headers) self.assertEqual(response.status_code, context_mock.status) self.assertEqual(response.headers.get('X-Clacks-Overhead'), 'Joe Armstrong') + + def test_rate_limit_is_logged_to_sentry(self): + url = reverse('api:github-webhook-filter', args=('id', 'token')) + payload = {} + headers = {'X-GitHub-Event': 'pull_request_review'} + with ( + mock.patch('urllib.request.urlopen') as urlopen, + mock.patch.object(GitHubWebhookFilterView, "logger") as logger, + ): + urlopen.side_effect = HTTPError(None, 429, 'Too Many Requests', {}, None) + logger.warning = mock.PropertyMock() + self.client.post(url, data=payload, headers=headers) + + logger.warning.assert_called_once() diff --git a/pydis_site/apps/api/views.py b/pydis_site/apps/api/views.py index 8a9eebd77f..2df93a6de6 100644 --- a/pydis_site/apps/api/views.py +++ b/pydis_site/apps/api/views.py @@ -1,6 +1,8 @@ import json +import logging import urllib.request from collections.abc import Mapping +from http import HTTPStatus from rest_framework import status from rest_framework.exceptions import ParseError @@ -254,6 +256,7 @@ class GitHubWebhookFilterView(APIView): authentication_classes = () permission_classes = () + logger = logging.getLogger("github_webhook_filter") def post(self, request: Request, *, webhook_id: str, webhook_token: str) -> Response: """Filter a webhook POST from GitHub before sending it to Discord.""" @@ -329,4 +332,10 @@ def send_webhook( with urllib.request.urlopen(request) as response: # noqa: S310 return (response.status, dict(response.getheaders()), response.read()) except urllib.error.HTTPError as err: # pragma: no cover + if err.code == HTTPStatus.TOO_MANY_REQUESTS: + self.logger.warning( + "We are being rate limited by Discord! Scope: %s, reset-after: %d", + headers.get("X-RateLimit-Scope"), + headers.get("X-RateLimit-Reset-After"), + ) return (err.code, dict(err.headers), err.fp.read())