fix: automatically clear auth token if refresh token is invalid #6760
Conversation
core/src/cloud/grow/auth.ts
Outdated
| @@ -86,8 +87,15 @@ export async function refreshAuthTokenAndWriteToConfigStore( | |||
| } | |||
|
|
|||
| log.debug({ msg: `Failed to refresh the token.` }) | |||
| if (err.message.toLowerCase().includes("invalid refresh token")) { | |||
There was a problem hiding this comment.
Can we also determine the token validity using the response status code?
| if (err.message.toLowerCase().includes("invalid refresh token")) { | |
| if (err.statusCode >= 400 && err.statusCode < 500) { |
There was a problem hiding this comment.
Maybe we should even test for 401 explicitly
There was a problem hiding this comment.
If we ever migrate to other kinds of tokens that need to be decoded, e.g. JWT tokens then we should also delete the token on 400 response though, so I think 400 <= code < 500 is fine
There was a problem hiding this comment.
Backend V2 did not return any status code for invalid token.
There was a problem hiding this comment.
We need to change the backend in that case, to make sure that it will emit correct status code in this case then. Maybe @hph can support you on this?
There was a problem hiding this comment.
The error returned is UNAUTHORIZED, which should map to 401. This status code is available in the httpStatus property, at least when using the frontend client, but I think it should be the same in core. Reference (see error shape above the linked table)
There was a problem hiding this comment.
I double-check that and did not see the status code in the TRPCClientError instance. The data property (that should contain httpStatus) was undefined, I'm not sure why.
There was a problem hiding this comment.
I found the original error code deep inside the error object.
There was a problem hiding this comment.
Is it maybe helpful to infer the specific error shape for the router? See also https://trpc.io/docs/client/vanilla/infer-types#infer-trpcclienterror-types
There was a problem hiding this comment.
I did it in feaa8d2 - we need the nested cause there. Not sure why the error is wrapped in such a way,
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: