fix: ID-2650 Handle unknown or invalid refresh token error #2399
Conversation
imx-mikhala
changed the title
| }).catch(); // User does not exist, don't initialise an eth signer | ||
| }).catch(() => { | ||
| // User does not exist, don't initialise an eth signer | ||
| }); |
There was a problem hiding this comment.
This was where the error was not being caught - catch requires a function to handle the error
☁️ Nx Cloud ReportCI is running/has finished running commands for commit bb83e34. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
| if (useCachedSession) { | ||
| if (error instanceof Error) trackError('passport', 'login', error); |
There was a problem hiding this comment.
I think where the trackError call was before made sense, but I think the right solution is to no longer surface unknown or invalid refresh token "errors" as errors. Maybe something like this:
} catch (error) {
// pseudo
if (errorIsUnknownOrInvalidRefreshToken) {
flow.addEvent('unknown or invalid refresh token');
} else {
trackError('passport', 'login', error);
}
There was a problem hiding this comment.
We only throw in the useCachedSession scenario which is why I moved the track here - do you think we should track other errors caught here even if they do not throw and just ignore any related to the invalid refresh? I suppose this would highlight any errors thrown here that we might not be handling so I think that makes sense.
Hi👋, please prefix this PR's title with:
breaking-change:if you have introduced modification that necessitates immediate adjustments by this SDK's users to their applications, clients, or integrations to avert disruptions to existing features or functionalities.feat:,fix:,refactor:,docs:, orchore:.Summary
Detail and impact of the change
Added
Changed
Deprecated
Removed
Fixed
Security
Anything else worth calling out?