Repository: MindscapeHQ/raygun4py

Issue:
Documentation for Custom grouping logic has incorrect definition of the callback parameter
The documentation for Custom grouping logic states that:

> The callback's one parameter is an instance of RaygunMessage (python[2/3]/raygunmsgs.py), and the callback should return a string.

But it appears that this is not the case. Instead it appears to be a dict object which is returned from `RaygunSender._transform_message` as a result of calling `utilities.filter_keys`. 

The code at [utilities.py Line 16](https://github.com/MindscapeHQ/raygun4py/blob/master/python2/raygun4py/utilities.py#L16) has this:
```
if isinstance(object, raygunmsgs.RaygunMessage):
        iteration_target = object.__dict__
```
which is the point where the RaygunMessage is converted to a dict.

The sample code `return raygun_message.get_error().message[:100]` therefore crashes with an AttributeError:
```
AttributeError: 'dict' object has no attribute 'get_error'
```

This however will work:
```
return raygun_message['details']['error'].message[:100]
```

The `raygun_message['details']['error']` accesses the instance of [RaygunErrorMessage](https://github.com/MindscapeHQ/raygun4py/blob/master/python2/raygun4py/raygunmsgs.py#L129)

Discussion:
@mduncan26: Hi @simonwaterer-c3, 

Thank you for raising this issue. I will raise this with the wider team so we can look to address this issue. I will update this thread once we have had a chance to investigate and produce a fix. Thank you for your patience.

We are also open to PRs for this repository. If you wish to implement a fix and have us review it for an earlier release, that is another option.

Thank you,
Mitchell.@simonwaterer-c3: Hi @mduncan26, 

No worries. I'm not waiting on a fix for this, just thought I'd raise it to you so that you and others are aware of it.

Cheers,
Simon @mduncan26: Thank you @simonwaterer-c3, we appreciated customers notifying us about these things 😄.

Solution patch:
diff --git a/README.rst b/README.rst
index 10da01c..9504eeb 100644
--- a/README.rst
+++ b/README.rst
@@ -353,9 +353,9 @@ Customer data can be passed in which will be displayed in the Raygun web app. Th
 Custom grouping logic
 ---------------------
 
-You can create custom exception grouping logic that overrides the automatic Raygun grouping by passing in a function that accepts one parameter using this function. The callback's one parameter is an instance of RaygunMessage (python[2/3]/raygunmsgs.py), and the callback should return a string.
+You can create custom exception grouping logic that overrides the automatic Raygun grouping by passing in a function that accepts one parameter using this function. The callback's one parameter is an instance of `RaygunMessage` (`python3/raygunmsgs.py`), and the callback should return a string.
 
-The RaygunMessage instance contains all the error and state data that is about to be sent to the Raygun API. In your callback you can inspect this RaygunMessage, hash together the fields you want to group by, then return a string which is the grouping key.
+The `RaygunMessage` instance contains all the error and state data that is about to be sent to the Raygun API. In your callback you can inspect this `RaygunMessage`, hash together the fields you want to group by, then return a string which is the grouping key.
 
 This string needs to be between 1 and 100 characters long. If the callback is not set or the string isn't valid, the default automatic grouping will be used.
 
diff --git a/python3/raygun4py/raygunmsgs.py b/python3/raygun4py/raygunmsgs.py
index 6af2054..e7489b2 100644
--- a/python3/raygun4py/raygunmsgs.py
+++ b/python3/raygun4py/raygunmsgs.py
@@ -15,7 +15,7 @@
     USE_MULTIPROCESSING = False
 
 import platform
-from datetime import datetime
+from datetime import datetime, timezone
 
 from raygun4py import http_utilities
 
@@ -129,15 +129,30 @@ def set_user(self, user):
 class RaygunMessage(object):
 
     def __init__(self):
-        self.occurredOn = datetime.utcnow()
+        self.occurredOn = datetime.now(timezone.utc)
         self.details = {}
 
+    def __copy__(self):
+        new_instance = RaygunMessage()
+        new_instance.details = self.details.copy()
+        new_instance.occurredOn = self.occurredOn
+        return new_instance
+
+    def copy(self):
+        return self.__copy__()
+
     def get_error(self):
         return self.details.get("error")
 
     def get_details(self):
         return self.details
 
+    def set_details(self, details):
+        self.details = details
+
+    def set_error(self, error):
+        self.details["error"] = error
+
 
 class RaygunErrorMessage(object):
     log = logging.getLogger(__name__)
diff --git a/python3/raygun4py/raygunprovider.py b/python3/raygun4py/raygunprovider.py
index 79a3ce2..f684f91 100644
--- a/python3/raygun4py/raygunprovider.py
+++ b/python3/raygun4py/raygunprovider.py
@@ -295,42 +295,44 @@ def _create_message(
         )
 
     def _transform_message(self, message):
+        message = message.copy()
         message = utilities.ignore_exceptions(self.ignored_exceptions, message)
 
         if message is not None:
-            message = utilities.filter_keys(self.filtered_keys, message)
-            message["details"]["groupingKey"] = utilities.execute_grouping_key(
+            details = message.get_details()
+            details = utilities.filter_keys(self.filtered_keys, details)
+            details["groupingKey"] = utilities.execute_grouping_key(
                 self.grouping_key_callback, message
             )
+            message.set_details(details)
 
         if self.before_send_callback is not None:
-            mutated_payload = self.before_send_callback(message["details"])
+            mutated_payload = self.before_send_callback(message.get_details())
 
             if mutated_payload is not None:
-                message["details"] = mutated_payload
+                message.set_details(mutated_payload)
             else:
                 return None
 
         return message
 
     def _post(self, raygunMessage):
+        raygunMessage = raygunMessage.copy()
         options = {
             "enforce_payload_size_limit": self.enforce_payload_size_limit,
             "log_payload_size_limit_breaches": self.log_payload_size_limit_breaches,
         }
 
         if (
-            isinstance(raygunMessage["details"]["error"], raygunmsgs.RaygunErrorMessage)
+            isinstance(raygunMessage.get_error(), raygunmsgs.RaygunErrorMessage)
             and "enforce_payload_size_limit" in options
             and options["enforce_payload_size_limit"] is True
         ):
-            error = jsonpickle.loads(
-                jsonpickle.dumps(raygunMessage["details"]["error"])
-            )
+            error = jsonpickle.loads(jsonpickle.dumps(raygunMessage.get_error()))
 
             error.check_and_modify_payload_size(options)
 
-            raygunMessage["details"]["error"] = error
+            raygunMessage.set_error(error)
 
         json = jsonpickle.encode(raygunMessage, unpicklable=False)
 
diff --git a/python3/raygun4py/utilities.py b/python3/raygun4py/utilities.py
index d2d6d0f..9c599a1 100644
--- a/python3/raygun4py/utilities.py
+++ b/python3/raygun4py/utilities.py
@@ -1,7 +1,5 @@
 import re
 
-from raygun4py import raygunmsgs
-
 
 def ignore_exceptions(ignored_exceptions, message):
     classname = message.get_error().get_classname()
@@ -11,11 +9,18 @@ def ignore_exceptions(ignored_exceptions, message):
     return message
 
 
-def filter_keys(filtered_keys, object):
-    iteration_target = object
+def filter_keys(filtered_keys, obj):
+    """
+    Filter keys from a dictionary.
+
+    Parameters:
+        filtered_keys (list): A list of keys to filter.
+        obj (dict): The dictionary to filter.
 
-    if isinstance(object, raygunmsgs.RaygunMessage):
-        iteration_target = object.__dict__
+    Returns:
+        dict: The filtered dictionary.
+    """
+    iteration_target = dict(obj)
 
     for key in iter(iteration_target.keys()):
         if isinstance(iteration_target[key], dict):
diff --git a/setup.py b/setup.py
index 5e36489..09b6aa6 100644
--- a/setup.py
+++ b/setup.py
@@ -9,7 +9,7 @@
     base_dir = "python3"
 
 exec(open("%s/raygun4py/version.py" % base_dir).read())
-requirements = ["jsonpickle >= 0.9.2", "blinker >= 1.3.0", "requests >= 2.9.1"]
+requirements = ["jsonpickle >= 4.0.4", "blinker >= 1.3.0", "requests >= 2.9.1"]
 
 dev_requirements = [
     "unittest2 >= 1.1.0",


Test patch:
diff --git a/python3/tests/test_raygunmsgs.py b/python3/tests/test_raygunmsgs.py
index 64f50cc..7bbf02f 100644
--- a/python3/tests/test_raygunmsgs.py
+++ b/python3/tests/test_raygunmsgs.py
@@ -159,6 +159,28 @@ def test_environment_variables_are_ignored(self):
             self.builder.raygunMessage.details["environment"]["environmentVariables"]
         )
 
+    def test_get_error(self):
+        self.builder.set_exception_details(
+            raygunmsgs.RaygunErrorMessage(Exception, None, None, {})
+        )
+        self.assertIsNotNone(self.builder.raygunMessage.get_error())
+
+    def test_get_details(self):
+        self.builder.set_exception_details(
+            raygunmsgs.RaygunErrorMessage(Exception, None, None, {})
+        )
+        self.assertIsNotNone(self.builder.raygunMessage.get_details())
+
+    def test_set_error(self):
+        message = raygunmsgs.RaygunMessage()
+        message.set_error("Error")
+        self.assertEqual(message.get_error(), "Error")
+
+    def test_set_details(self):
+        message = raygunmsgs.RaygunMessage()
+        message.set_details({"foo": "bar"})
+        self.assertEqual(message.get_details(), {"foo": "bar"})
+
 
 class TestRaygunErrorMessage(unittest.TestCase):
     ONEHUNDRED_AND_FIFTY_KB = 150 * 1024
diff --git a/python3/tests/test_raygunprovider.py b/python3/tests/test_raygunprovider.py
index 1540e50..5f5462f 100644
--- a/python3/tests/test_raygunprovider.py
+++ b/python3/tests/test_raygunprovider.py
@@ -126,6 +126,9 @@ class TestGroupingKey(unittest.TestCase):
     def the_callback(self, raygun_message):
         return self.key
 
+    def the_callback_with_error(self, raygun_message):
+        return raygun_message.get_error().message[:100]
+
     def create_dummy_message(self):
         self.sender = raygunprovider.RaygunSender("apikey")
 
@@ -134,11 +137,17 @@ def create_dummy_message(self):
         msg.set_exception_details(errorMessage)
         return msg.build()
 
+    def test_message_with_error(self):
+        msg = self.create_dummy_message()
+        self.sender.on_grouping_key(self.the_callback_with_error)
+        msg = self.sender._transform_message(msg)
+        self.assertEqual(msg.get_details()["groupingKey"], "Exception: None")
+
     def test_groupingkey_is_not_none_with_callback(self):
         msg = self.create_dummy_message()
         self.sender.on_grouping_key(self.the_callback)
         self.key = "foo"
-        self.sender._transform_message(msg)
+        msg = self.sender._transform_message(msg)
 
         self.assertIsNotNone(msg.get_details()["groupingKey"])
 
@@ -146,7 +155,7 @@ def test_groupingkey_is_set_with_callback(self):
         msg = self.create_dummy_message()
         self.sender.on_grouping_key(self.the_callback)
         self.key = "foo"
-        self.sender._transform_message(msg)
+        msg = self.sender._transform_message(msg)
 
         self.assertEqual(msg.get_details()["groupingKey"], "foo")
 
@@ -154,7 +163,7 @@ def test_groupingkey_is_string_with_callback(self):
         msg = self.create_dummy_message()
         self.sender.on_grouping_key(self.the_callback)
         self.key = "foo"
-        self.sender._transform_message(msg)
+        msg = self.sender._transform_message(msg)
 
         self.assertIsInstance(msg.get_details()["groupingKey"], str)
 
@@ -162,7 +171,7 @@ def test_groupingkey_is_none_when_not_string_returned_from_callback(self):
         msg = self.create_dummy_message()
         self.sender.on_grouping_key(self.the_callback)
         self.key = object
-        self.sender._transform_message(msg)
+        msg = self.sender._transform_message(msg)
 
         self.assertIsNone(msg.get_details()["groupingKey"])
 
@@ -170,7 +179,7 @@ def test_groupingkey_is_none_when_empty_string_returned_from_callback(self):
         msg = self.create_dummy_message()
         self.sender.on_grouping_key(self.the_callback)
         self.key = ""
-        self.sender._transform_message(msg)
+        msg = self.sender._transform_message(msg)
 
         self.assertIsNone(msg.get_details()["groupingKey"])
 
@@ -182,7 +191,7 @@ def test_groupingkey_is_set_when_ok_length_string_returned_from_callback(self):
         for i in range(0, 99):
             self.key += "a"
 
-        self.sender._transform_message(msg)
+        msg = self.sender._transform_message(msg)
         self.assertEqual(msg.get_details()["groupingKey"], self.key)
 
     def test_groupingkey_is_none_when_too_long_string_returned_from_callback(self):
@@ -193,7 +202,7 @@ def test_groupingkey_is_none_when_too_long_string_returned_from_callback(self):
         for i in range(0, 100):
             self.key += "a"
 
-        self.sender._transform_message(msg)
+        msg = self.sender._transform_message(msg)
         self.assertIsNone(msg.get_details()["groupingKey"])
 
 
diff --git a/python3/tests/test_utilities.py b/python3/tests/test_utilities.py
index f745a89..daa1fa2 100644
--- a/python3/tests/test_utilities.py
+++ b/python3/tests/test_utilities.py
@@ -7,14 +7,14 @@ class TestRaygunUtilities(unittest.TestCase):
     def test_filter_keys(self):
         test_obj = {"foo": "bar", "baz": "qux"}
 
-        utilities.filter_keys(["foo"], test_obj)
+        test_obj = utilities.filter_keys(["foo"], test_obj)
 
         self.assertEqual(test_obj["foo"], "<filtered>")
 
     def test_filter_keys_recursive(self):
         test_obj = {"foo": "bar", "baz": "qux", "boo": {"foo": "qux"}}
 
-        utilities.filter_keys(["foo"], test_obj)
+        test_obj = utilities.filter_keys(["foo"], test_obj)
 
         self.assertEqual(test_obj["foo"], "<filtered>")
         self.assertEqual(test_obj["boo"]["foo"], "<filtered>")
@@ -22,7 +22,7 @@ def test_filter_keys_recursive(self):
     def test_filter_keys_with_wildcard(self):
         test_obj = {"foobr": "bar", "foobz": "baz", "fooqx": "foo", "baz": "qux"}
 
-        utilities.filter_keys(["foo*"], test_obj)
+        test_obj = utilities.filter_keys(["foo*"], test_obj)
 
         self.assertEqual(test_obj["foobr"], "<filtered>")
         self.assertEqual(test_obj["foobz"], "<filtered>")