pyrollbar
pyrollbar copied to clipboard
In rollbar.report_message's extra_data kwarg, "body" being reserved leads to surprising behavior
Per this comment on rollbar.report_message: https://github.com/rollbar/pyrollbar/blob/33ef2e723a33d09dd6302f978f4a3908be95b9d2/rollbar/init.py#L427 the body key in the extra_data kwarg is "reserved". I discovered the hard way that, if you happen to pass a dict with a body key, the message you intended to report is replaced with that raw JSON. This was quite difficult to track down, as the behavior did not communicate the "reserved"ness of body - nor is it mentioned in the documentation; only in the docstring.
I'd like to request that this reserved key be changed to something that's harder to accidentally collide with - or at least for pyrollbar to raise an exception or print a warning or something letting me know what I did.
Thanks @brettdh; that does sound like quite the rabbit hole...
@rokob thoughts? I think this behavior comes from here:
https://github.com/rollbar/pyrollbar/blob/33ef2e723a33d09dd6302f978f4a3908be95b9d2/rollbar/init.py#L754
A few ideas:
- We could change the API format so that the reserved 'body' field has a different name, i.e. '$body', or we could move it to a different part of the payload
- We could have extra_data go in a nested dict, i.e.
data['body']['message']['extra']instead of merging it intodata['body']['message'] - We could rename the
extra_data['body']key so it doesn't override the reserved body
After typing this out, (2) seems like the best way to go. The downside of that is that it would make the key a bit longer when it appears in the rollbar UI, but that seems like an OK compromise to avoid the nasty in-band signaling problem we have right now.
I think 2 sounds reasonable. In other SDKs we don't merge extra data into data.body.message but into data.custom basically at the top level