Conversation
yanniszark
left a comment
yanniszark
left a comment
There was a problem hiding this comment.
Thanks @ryandawsonuk!
Overall looks good, just a couple of nits.
After that and a rebase we should be good to go.
handlers.go
Outdated
| logger.Debug("Existing user session "+userID) | ||
| returnStatus(w, http.StatusOK, "OK") | ||
| return | ||
| } else { |
There was a problem hiding this comment.
I think this else is not needed.
After this point, you know it's a new session anyway
There was a problem hiding this comment.
The purpose of the else is to log the session struct.
There was a problem hiding this comment.
What I mean is that at that point, you know that this is a new session, because we return inside the if.
Thus, the else is not necessary, you could just log it.
There was a problem hiding this comment.
I understand now. Yes that's clearer to have it in the main flow rather than the else.
handlers.go
Outdated
| logger.Errorf("Failed to save state in store: %v", err) | ||
| returnStatus(w, http.StatusInternalServerError, "Failed to save state in store.") | ||
| return | ||
| } else { |
There was a problem hiding this comment.
Same, is else needed here?
There was a problem hiding this comment.
This also logs the session struct and means it can be seen what has changed in since the struct was created.
ryandawsonuk
force-pushed
the
2-loglevels
branch
from
5b5c7c3 to
b2abdda
Compare
2 participants
to help debug #2
An example of debug logging for a new session:
