Merge pull request #31 from larsbutler/unbreak-notify

Fix `Airbrake.notify` regression

airbrake/notifier.py

@@ -186,14 +186,24 @@ def log(self, exc_info=None, message=None, filename=None,
                    'notifier': self.notifier,
                    'environment': environment,
                    'session': session}
-        return self.notify(json.dumps(payload, cls=utils.FailProofJSONEncoder))
+        return self.notify(payload)
 
     def notify(self, payload):
-        """Post the current errors payload body to airbrake.io."""
+        """Post the current errors payload body to airbrake.io.
+
+        :param dict payload:
+            Notification payload, in a dict/object form. The notification
+            payload will ultimately be sent as a JSON-encoded string, but here
+            it still needs to be in object form.
+        """
         headers = {'Content-Type': 'application/json'}
         api_key = {'key': self.api_key}
-        response = requests.post(self.api_url, data=payload,
-                                 headers=headers, params=api_key)
+        response = requests.post(
+            self.api_url,
+            data=json.dumps(payload, cls=utils.FailProofJSONEncoder,
+                            sort_keys=True),
+            headers=headers,
+            params=api_key)
         response.raise_for_status()
         return response
 

tests/test_notifier.py

@@ -1,7 +1,6 @@
 import airbrake
 import mock
 import unittest
-import json
 
 from airbrake.notifier import Airbrake
 
@@ -10,7 +9,6 @@ class TestAirbrakeNotifier(unittest.TestCase):
 
     def _create_notify(test, exception, session={}, environment={}, **params):
         def notify(self, payload):
-            payload = json.loads(payload)
             test.assertEqual(session, payload['session'])
             test.assertEqual(environment, payload['environment'])
             test.assertEqual(str(exception), payload['errors'][0]['message'])
@@ -67,5 +65,24 @@ def __repr__(self):
             extra = {'very': 'important', 'jsonify': non_serializable}
             self.logger.exception(Exception(msg), extra=extra)
 
+    def test_notify_payload(self):
+        # 1d7b191a2cc3b53f76f12d09f68c857183859e08 broke the `notify interface
+        # by making the assumption that the payload sent to `notify` is already
+        # encoded as a JSON string.
+        # This test ensures that such a regression can't happen again.
+        with mock.patch('requests.post') as requests_post:
+            ab = Airbrake(project_id=1234, api_key='fake', environment='test')
+            payload = dict(foo=1, bar=2)
+            ab.notify(payload)
+
+            expected_call_args = mock.call(
+                'https://airbrake.io/api/v3/projects/1234/notices',
+                data='{"bar": 2, "foo": 1}',
+                headers={'Content-Type': 'application/json'},
+                params={'key': 'fake'}
+            )
+            self.assertEqual(expected_call_args, requests_post.call_args)
+
+
 if __name__ == '__main__':
     unittest.main()