Conversation
davehwly
left a comment
davehwly
left a comment
There was a problem hiding this comment.
fill in PR template with what & why
|
|
||
| if (!isAuthenticated || !!accessToken) return | ||
| if (!isAuthenticated || !!accessToken) { | ||
| errorLog('Not authenticate but access token exists.') |
There was a problem hiding this comment.
you can't tell if a token exists from this lazy evaluation can you?
There was a problem hiding this comment.
yeah but i think we can just say "not authenticated"?
There was a problem hiding this comment.
Are we adding nuxt-oauth - prefix to all the logs?
| } else { | ||
| errorLog("nuxt-oauth - session token not found") | ||
| return {} | ||
| } |
There was a problem hiding this comment.
If we remove the nesting would that improve readability?
if (!token) errorLog("nuxt-oauth - session token not found")
return token || {}
| await this.saveData({ accessToken: existingToken }) | ||
| } | ||
| } catch (e) { | ||
| errorLog("nuxt-oauth - saveData failed, clear the session") |
There was a problem hiding this comment.
Would logging the error also be helpful here?
There was a problem hiding this comment.
yeah I think @naveen-soho we need to make sure these errorLogs are actually error situations. so they should be instances where we're throwing or catching errors and also making sure the errors are logged too
so for example if they don't have an accessToken, is that a situation that should never occur? incase we need an errorLog. If it could reasonably occur normally we don't need to log
we want to make sure abnormal situations are logged and captured in raygun etc
|
|
||
| if (!isAuthenticated || !!accessToken) return | ||
| if (!isAuthenticated || !!accessToken) { | ||
| errorLog('Not authenticate but access token exists.') |
There was a problem hiding this comment.
Are we adding nuxt-oauth - prefix to all the logs?
3 participants
What/Why?
Added missing/uncaught OAuth errors logs and issues like JS-1029 can be identified at an earlier stage