joshuap
commented
8 months ago @subzero10 @Kelvin4664 can you take a look at this?
Closed payala closed 7 months ago
joshuap
commented
8 months ago @subzero10 @Kelvin4664 can you take a look at this?
Kelvin4664
commented
8 months ago Hi @payala I can indeed see from the screenshot that the local variable "_" is a dictionary whose first key is a tuple. I am wondering why it is so in the first place. Can you provide a minimal version of your code to help me reproduce the error? I am trying to confirm the root cause before i work on a patch.
subzero10
commented
8 months ago Hey @Kelvin4664, it seems that the local variable _ is a function. What if we filter out functions before trying to encode as json?
Hey @payala, while we work out the fix, you may disable local_variables collection through configuration.
Kelvin4664
commented
8 months ago Hey @subzero10
It goes slightly beyond that. We are correctly encoding the values using a custom encoder to eliminate unserializable values.
if you look at the screenshots and traceback closely, you'd notice the problem here is with the dictionary key, not the value. The value of _ looks like this:
'_'= {(function, ()): SessionObject}
subzero10
commented
8 months ago OK, now I get it, thanks @Kelvin4664.
So:
Is the above correct? If yes, what if we traverse the dictionary recursively and apply a transformation to the key if necessary (i.e. change tuple to a string)?
payala
commented
8 months ago Indeed, @Kelvin4664 the problem has to do with the key being a tuple, that's what gives the error. I did have a look to find out why there is this tuple key in the first place, but I didnt find out why or who is injecting this key in there.
The '_' variable is something used in interactive sessions, but in this context it's not obvious to me what this is.
I will try to come up with a minimal version to reproduce the error. For now, I do have the local_variable collection disabled as a workaround.
Thanks for having a look at this guys!
payala
commented
8 months ago OK, bd news, sorry to inform that I'm not able to reproduce it anymore. The endpoint I was using is having a dependency on the "get_db" function.
I'm not sure if this underscore variable is where dependencies are stored through the request processing chain. I had a look at FastAPI code, but I wasn't able to find anything meaningful.
subzero10
commented
7 months ago Hey @payala, thanks for the update. We will attempt to correct this scenario by either ignoring dictionary keys that are tuples or adding them to the JSON with the key being a string.
I think this is probably a honeybadger edge-case bug.
There is an endpoint which I’m hitting badly intentionally, it should return a 422 because I’m passing 1234 instead of a valid guid.
It works well in my local test environment because hb is disabled in local, but in production, where honey badger is enabled it results in a 500 error, and no notice is sent to hb, because it is failing in the notification sending itself. This is the error trace I see in the server:
I can perfectly reproduce this in my local environment. I have been having a look, but I’m a bit lost at what is happening. I’ve narrowed down the last error and I see that indeed there is a dict with a key which is hard to encode.
What I see is that finally, at encoder.py, line 200 (actually seems to be line 199 for me, at
chunks = self.iterencode(o, _one_shot=True)I see that it fails if I try
self.iterencode(o["request"]["local_variables"]["_"], _one_shot=True)This ‘_’ key looks like this:
Why this appears in the
local_variables, of course I have no clue.So, then this raises an exception inside honey badger trying to send the notification to the service, which explains why no error appears in hb itself. If I disable hb, I get the expected 422 response.
In payload.py, line 118:
I can see that the localvariables do indeed have a `''` variable in it:
I have worked around this by disabling local variable collection in honeybadger config in the meanwhile until a fix is found.