diff options
author | 2024-04-01 13:59:16 +0100 | |
---|---|---|
committer | 2024-04-01 13:59:16 +0100 | |
commit | 5d4e72910b85b0961b33959dfb606d26de69b98a (patch) | |
tree | 0ab1f6022c92d903aeb6ca4d9a228c7f01505da1 /pydis_site/apps | |
parent | Update Python version requirements in contributing guide (#1283) (diff) | |
parent | Log failed webhook attempts to stderr in GitHub Webhook Filter (diff) |
Merge pull request #1281 from python-discord/jb3/github-webhook-no-status
Don't propagate the status received from Discord to GitHub Webhook
Diffstat (limited to 'pydis_site/apps')
-rw-r--r-- | pydis_site/apps/api/tests/test_github_webhook_filter.py | 20 | ||||
-rw-r--r-- | pydis_site/apps/api/views.py | 23 |
2 files changed, 38 insertions, 5 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 8ca60511..649a9e64 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 @@ class GitHubWebhookFilterAPITests(APITestCase): 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')) @@ -60,3 +62,17 @@ class GitHubWebhookFilterAPITests(APITestCase): 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 1fa3efc2..a3b0016c 100644 --- a/pydis_site/apps/api/views.py +++ b/pydis_site/apps/api/views.py @@ -303,9 +303,26 @@ class GitHubWebhookFilterView(APIView): (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) - return Response(data=body, headers=headers, status=response_status) + + 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_decoded, + "headers": headers, + } + + return Response(response_body) def send_webhook( self, |