diff --git a/bugsnag/delivery.py b/bugsnag/delivery.py index acd2da2..6dc8cd0 100644 --- a/bugsnag/delivery.py +++ b/bugsnag/delivery.py @@ -93,7 +93,6 @@ def deliver_sessions(self, config, payload: Any, options=None): options = {} options['endpoint'] = config.session_endpoint - options['success'] = 202 self.deliver(config, payload, options) @@ -151,10 +150,14 @@ def request(): status = resp.getcode() if 'success' in options: - success = options['success'] + # if an expected status code has been given then it must match + # exactly with the actual status code + success = status == options['success'] else: - success = 200 - if status != success: + # warn if we don't get a 2xx status code by default + success = status >= 200 and status < 300 + + if not success: config.logger.warning( 'Delivery to %s failed, status %d' % (uri, status) ) @@ -184,12 +187,16 @@ def request(): response = requests.post(uri, **req_options) status = response.status_code + if 'success' in options: - success = options['success'] + # if an expected status code has been given then it must match + # exactly with the actual status code + success = status == options['success'] else: - success = requests.codes.ok + # warn if we don't get a 2xx status code by default + success = status >= 200 and status < 300 - if status != success: + if not success: config.logger.warning( 'Delivery to %s failed, status %d' % (uri, status) ) diff --git a/tests/test_delivery.py b/tests/test_delivery.py index 591bcfd..d86a6a5 100644 --- a/tests/test_delivery.py +++ b/tests/test_delivery.py @@ -1,9 +1,12 @@ import pytest import warnings import sys +import logging +from unittest.mock import Mock from bugsnag import Configuration from bugsnag.delivery import ( + Delivery, UrllibDelivery, RequestsDelivery, create_default_delivery, @@ -66,6 +69,71 @@ def test_misconfigured_sessions_endpoint_sends_warning(self): self.assertEqual(1, len(warn)) self.assertEqual(0, len(self.server.events_received)) + def test_does_not_warn_on_200_status_code(self): + delivery = Mock(wraps=create_default_delivery()) + logger = Mock(spec=logging.Logger) + + self.config.configure(delivery=delivery, logger=logger) + + self.server.respond_to_next_request_with(200) + + delivery.deliver(self.config, '{"apiKey":"aaab"}') + + assert logger.warning.call_count == 0 + assert len(self.server.events_received) == 1 + + def test_does_not_warn_on_202_status_code(self): + delivery = Mock(wraps=create_default_delivery()) + logger = Mock(spec=logging.Logger) + + self.config.configure(delivery=delivery, logger=logger) + + self.server.respond_to_next_request_with(202) + + delivery.deliver(self.config, '{"apiKey":"aaab"}') + + print(logger.warning.assert_not_called()) + assert logger.warning.call_count == 0 + assert len(self.server.events_received) == 1 + + def test_warns_on_400_status_code_for_events(self): + delivery = Mock(wraps=create_default_delivery()) + logger = Mock(spec=logging.Logger) + + self.config.configure(delivery=delivery, logger=logger) + + self.server.respond_to_next_request_with(300) + + delivery.deliver(self.config, '{"apiKey":"aaab"}') + + expected = 'Delivery to %s failed, status %d' % ( + self.config.endpoint, + 300 + ) + + logger.warning.assert_called_once_with(expected) + + assert len(self.server.events_received) == 1 + + def test_warns_on_300_status_code_for_sessions(self): + delivery = Mock(wraps=create_default_delivery()) + logger = Mock(spec=logging.Logger) + + self.config.configure(delivery=delivery, logger=logger) + + self.server.respond_to_next_request_with(300) + + delivery.deliver_sessions(self.config, '{"apiKey":"aaab"}') + + expected = 'Delivery to %s failed, status %d' % ( + self.config.session_endpoint, + 300 + ) + + logger.warning.assert_called_once_with(expected) + + assert len(self.server.sessions_received) == 1 + def test_it_calls_post_delivery_callback(self): callback_was_called = False diff --git a/tests/utils.py b/tests/utils.py index d17e138..38009bf 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -81,6 +81,7 @@ def __init__(self, wait_for_duplicate_requests: bool): self.sessions_received = [] self.paused = False self.wait_for_duplicate_requests = wait_for_duplicate_requests + self._response_codes = [] class Handler(SimpleHTTPRequestHandler): def do_POST(handler): @@ -114,7 +115,12 @@ def do_POST(handler): 'unknown endpoint requested: ' + handler.path ) - handler.send_response(200) + try: + code = self._response_codes.pop() + except IndexError: + code = 200 + + handler.send_response(code) handler.end_headers() return () @@ -175,6 +181,9 @@ def sent_report_count(self) -> int: def sent_session_count(self) -> int: return len(self.sessions_received) + def respond_to_next_request_with(self, status_code: int) -> None: + self._response_codes.append(status_code) + class ScaryException(Exception): class NestedScaryException(Exception):