Skip to content
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

Skip deserialisation if no session content #5

Closed
bigpresh opened this issue Aug 9, 2013 · 4 comments
Closed

Skip deserialisation if no session content #5

bigpresh opened this issue Aug 9, 2013 · 4 comments

Comments

@bigpresh
Copy link

bigpresh commented Aug 9, 2013

No check is made to see if any session data was actually retrieved before trying to deserialise it, leading to the following mildly irritating error in the logs:

[1078261]   (Fri Aug  9 12:19:13 2013 from unknown)
    warn @0.110656> [hit #285]Could not retrieve session ID 81841834147620443673007375773661338 - malformed JSON string, neither array, object, number, string or atom, at character offset 0 (before "(end of string)") at /opt/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/JSON.pm line 171. in /opt/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/Dancer/Session/DBI.pm l. 163

It would make sense, I think, to check whether you got any session data, and if not, bail gracefully there before attempting to deserialise.

@LoonyPandora
Copy link
Owner

The module already does this, and the warning you see is the "bailing gracefully" when the session could not be deserialized : https://github.com/LoonyPandora/Dancer-Session-DBI/blob/master/lib/Dancer/Session/DBI.pm#L187-L192

I don't think there is a more graceful way to do it.

@bigpresh
Copy link
Author

I wonder if a warning is really necessary? Not finding a session with a given session ID will be a pretty common thing in a busy app - just people hitting the app with an expired session ID, say. Maybe it should be at debug level instead?

(Also, checking for session data before attempting to deserialize it would at least silence the JSON deserialisation part of the error, which is the scarier part which makes it sound like something is broken, rather than a routine situation being handled as designed.)

@LoonyPandora
Copy link
Owner

Hmm, yeah making it a debug level warning would be nicer. I'll get on it :)

@LoonyPandora
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants