From 5d01a07a4287ee0fd1fd441d72da665a7704a382 Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 01:54:10 +0100 Subject: [PATCH 1/4] Don't propagate the status received from Discord to GitHub Webhook --- pydis_site/apps/api/views.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pydis_site/apps/api/views.py b/pydis_site/apps/api/views.py index 1fa3efc28..ab7a5cf3c 100644 --- a/pydis_site/apps/api/views.py +++ b/pydis_site/apps/api/views.py @@ -305,7 +305,14 @@ def post(self, request: Request, *, webhook_id: str, webhook_token: str) -> Resp ) headers.pop('Connection', None) headers.pop('Content-Length', None) - return Response(data=body, headers=headers, status=response_status) + + response_body = { + "original_status": response_status, + "data": body.decode("utf-8"), + "headers": headers, + } + + return Response(response_body) def send_webhook( self, From d0e9469823ae66b99a5b7aee0ca98ac9814b5102 Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 02:05:46 +0100 Subject: [PATCH 2/4] Update GitHub Filter endpoint tests for new response --- pydis_site/apps/api/tests/test_github_webhook_filter.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 8ca605119..a2a00d4cd 100644 --- a/pydis_site/apps/api/tests/test_github_webhook_filter.py +++ b/pydis_site/apps/api/tests/test_github_webhook_filter.py @@ -44,8 +44,10 @@ def test_accepts_interesting_events(self): context_mock.read.return_value = b'{"status": "ok"}' 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') + response_body = response.json() + self.assertEqual(response.status_code, 200) + self.assertEqual(response_body.get("headers", {}).get("X-Clacks-Overhead"), 'Joe Armstrong') + self.assertEqual(response_body.get("original_status"), 299) def test_rate_limit_is_logged_to_sentry(self): url = reverse('api:github-webhook-filter', args=('id', 'token')) From 02787cd74a60df0d7254d49716764f97d034a200 Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 12:45:50 +0100 Subject: [PATCH 3/4] No need to remove headers in GitHub Filter Endpoint --- pydis_site/apps/api/views.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pydis_site/apps/api/views.py b/pydis_site/apps/api/views.py index ab7a5cf3c..787f8811d 100644 --- a/pydis_site/apps/api/views.py +++ b/pydis_site/apps/api/views.py @@ -303,8 +303,6 @@ def post(self, request: Request, *, webhook_id: str, webhook_token: str) -> Resp (response_status, headers, body) = self.send_webhook( webhook_id, webhook_token, request.data, dict(request.headers), ) - headers.pop('Connection', None) - headers.pop('Content-Length', None) response_body = { "original_status": response_status, From 3d04acc24d32d82bf1d817adaae9c65a0cd7ef92 Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Mon, 1 Apr 2024 12:59:00 +0100 Subject: [PATCH 4/4] Log failed webhook attempts to stderr in GitHub Webhook Filter --- .../apps/api/tests/test_github_webhook_filter.py | 14 ++++++++++++++ pydis_site/apps/api/views.py | 14 +++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) 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 a2a00d4cd..649a9e644 100644 --- a/pydis_site/apps/api/tests/test_github_webhook_filter.py +++ b/pydis_site/apps/api/tests/test_github_webhook_filter.py @@ -62,3 +62,17 @@ def test_rate_limit_is_logged_to_sentry(self): self.client.post(url, data=payload, headers=headers) logger.warning.assert_called_once() + + def test_other_error_is_logged(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, 451, 'Unavailable For Legal Reasons', {}, 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 787f8811d..a3b0016c1 100644 --- a/pydis_site/apps/api/views.py +++ b/pydis_site/apps/api/views.py @@ -304,9 +304,21 @@ def post(self, request: Request, *, webhook_id: str, webhook_token: str) -> Resp webhook_id, webhook_token, request.data, dict(request.headers), ) + body_decoded = body.decode("utf-8") + + if ( + not (status.HTTP_200_OK <= response_status < status.HTTP_300_MULTIPLE_CHOICES) + and response_status != status.HTTP_429_TOO_MANY_REQUESTS + ): + self.logger.warning( + "Failed to send GitHub webhook to Discord. Response code %d, body: %s", + response_status, + body_decoded, + ) + response_body = { "original_status": response_status, - "data": body.decode("utf-8"), + "data": body_decoded, "headers": headers, }