stormpath-sdk-java icon indicating copy to clipboard operation
stormpath-sdk-java copied to clipboard

Please log "invalid JWT in refresh_token cookie" as just an error (without an exception)

Open george-hawkins-work opened this issue 9 years ago • 4 comments

Currently we run tests where we do things such as create and delete users and test that the given user can no longer access anything.

When we do this we see a reasonable Stormpath error message in our logs:

Encountered invalid JWT in refresh_token cookie. We will now delete both the access and refresh cookies for safety.

These messages though are always accompanied by an exception stack trace. But this is a handled situation - the system knows what's going on and has reacted to it.

Generally I think logging stack traces should be reserved for situations where the system isn't quite sure what's happened and is logging where things went wrong so that someone can debug the point of failure (possibly at an unknown point far away from where the exception has been caught).

Currently we treat exceptions in our logs as indications that something is probably broken. This is not the case in this situation. Our QA investigates any exceptions that get logged - so currently this exception has to be put in a special category of expected exceptions.

Perhaps log the message at log level ERROR and the exception at log level DEBUG if you're worried that the exception may not always indicate a JWT issue (you are after all catching Exception e rather than a JWT specific exception).

george-hawkins-work avatar Oct 27 '16 09:10 george-hawkins-work

Hey @george-hawkins-aa. Thanks for reporting this. I totally agree with you, an exception is not reasonable in this case. We will improve this for sure. Thanks!

mrioan avatar Oct 27 '16 20:10 mrioan

This error appears to be a client-side issue. So it seems more appropriate to log the message at a lower level altogether; certainly no higher than WARN. If you agree I'm willing to open a PR.

Stephan202 avatar Mar 08 '17 11:03 Stephan202

@Stephan202 We'd be happy to review a PR.

mraible avatar Mar 08 '17 12:03 mraible

@mraible: I filed #1313. Happy to tweak the PR as required; I can see there may be various opinions/preferences around this topic.

Stephan202 avatar Mar 08 '17 20:03 Stephan202