pyrollbar icon indicating copy to clipboard operation
pyrollbar copied to clipboard

In rollbar.report_message's extra_data kwarg, "body" being reserved leads to surprising behavior

Open brettdh opened this issue 6 years ago • 2 comments

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.

brettdh avatar Mar 14 '19 14:03 brettdh

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:

  1. 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
  2. We could have extra_data go in a nested dict, i.e. data['body']['message']['extra'] instead of merging it into data['body']['message']
  3. 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.

brianr avatar Mar 14 '19 22:03 brianr

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

rokob avatar Mar 14 '19 22:03 rokob