-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Log session when session unpickling fails #34
base: master
Are you sure you want to change the base?
Conversation
@@ -154,9 +158,13 @@ private void deserializeConnector(Iterator<Op> opIterator) { | |||
case "is_public": | |||
isPublic = deserializeBooleanField(opIterator); | |||
break; | |||
default: | |||
log.warn("Unexpected field name in connector: " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use parameterized logging:
} | ||
} catch (Exception e) { | ||
log.error("Exception while deserializing: {}", fieldName); | ||
log.error("Pickled Session: " + b64Session); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above.
Also, I would assume we would rather want to surround the init()
in the constructor with a try block and log this at ERROR level if an exception is thrown there, no?
Previously, to debug an "unpickling" error, we had to dig through Redis to find the session which had created the error. With these changes, exceptions thrown during the unpickling process are caught and the failing session is logged. We also added some logic to give more useful errors in the case where a
BINGET
requests a memoized item which is not present in thememo
object.