Return data from Reauthentication middleware when shouldRetry: false#9961
Conversation
…equest.resolve promise
|
|
luacmartins
left a comment
luacmartins
left a comment
There was a problem hiding this comment.
LGTM! Thanks for fixing this!
|
|
||
| function invalidateAuthToken() { | ||
| NetworkStore.setAuthToken('pizza'); | ||
| Onyx.merge(ONYXKEYS.SESSION, {authToken: 'pizza'}); |
There was a problem hiding this comment.
pizza? lol, did you mean to clear it here instead or actually leave it as pizza?
There was a problem hiding this comment.
lol seems like we were already doing that in TestToolsMenu haha. I like it :)
There was a problem hiding this comment.
🍕 if we set it as null the user would be logged out or other weird things. So "bad authToken" vs. "no authToken" is what we want.
There was a problem hiding this comment.
cool, can that be added as a comment so that its not easily forgotten for the next person updating this block.
There was a problem hiding this comment.
Anyway, im not going to block since adding a comment is a NAB. Going to approve and merge this since its the only thing blocking deploy.
…cationIssue Return `data` from `Reauthentication` middleware when `shouldRetry: false` (cherry picked from commit dd9c8f5)
…9961 🍒 Cherry pick PR #9961 to staging 🍒
|
Nice, thanks for this! |
|
🚀 Deployed to production by @chiragsalian in version: 1.1.84-13 🚀
|
5 participants
Details
cc @yuwenmemon @luacmartins
When we switched from using
makeRequestWithSideEffects()instead of the "main queue" (a.k.a. deprecatedAPI) for authenticating Pusher we changed the way those responses are handled when theauthTokenexpires. Previously, arequest.resolvefunction was provided to the request object so the original response could be processed by the original caller whenshouldRetry: falseistrue. Since we do not pass this promise the response data is never returned and Pusher's custom authorizer can't handle the response or reauthentication attempts.Fixed Issues
$ #9958
Tests
There isn't a clear natural reproduction for the linked issue other than waiting a long time for the
authTokento expire and I am mostly following the trail of logs, but manually killing theauthTokenseems to work and we can do it via the "Preferences" on dev or staging.See QA Steps
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarhave been tested & I retested again)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
getPusherInstance()in the JS console and verifying that the private user channel issubscribed: trueIf this test is repeated without the line of code changing to return the data we will no longer be subscribed.
Screenshots
Web
Mobile Web
Desktop
iOS
Android