Remove unserializable data from metadata in Log records#2469
Conversation
There was a problem hiding this comment.
would it be better to to a str or them, so we don;'t completely lose the content?
There was a problem hiding this comment.
except the problem is that I am not sure if this guarantees that that is possible. I think args can literally contain any python object and so it might be possible that even calling str on it might fail right? Given that this leads to a database error I'd rather err on the safe side given that this information is more or less duplicated because it will often already be formatted into the message of the record itself.
There was a problem hiding this comment.
I think that any python object can be converted with str (unless you have an exception in the str method by mistake).then maybe it's useless if it only gives the class name, or it's redundant. do you have an example where you could check the str value in the relevant case, eg in the test you wrote, as well as the other Metadata stored?
There was a problem hiding this comment.
For this test it works, but I just was a bit afraid that there might be other cases where that approach wouldn't work and given the resulting excepting is pretty nasty, i.e. if this happens in a daemon worker, the whole daemon worker becomes defunct as its database session becomes corrupted. Therefore I was playing on the safe side. But I changed it to stringify the content. Let's see how that goes
1ec6dae to
24b5350
Compare
If unserializable data, often found in the `exc_info` or `args` keys, is not removed from the metadata of a log record, a database exception will be thrown.
24b5350 to
bc39e5a
Compare
|
@giovannipizzi let's merge it like this and I will report back when I encounter new issues with this approach |
Fixes #2363
If unserializable data, often found in the
exc_infoorargskeys, isnot removed from the metadata of a log record, a database exception will
be thrown.