[Session] Reset global agent on expire #3838
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Your Render PR Server URL is https://social-app-pr-3838.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-coq75pud3nmc73b1o7p0. |
gaearon
commented
May 3, 2024
| did: undefined, | ||
| } | ||
| : s.currentAgentState, | ||
| currentAgentState: s.currentAgentState, |
There was a problem hiding this comment.
This is now handled by the above setState so we can just carry over its state.
|
haileyok
approved these changes
May 3, 2024
estrattonbailey
added a commit
that referenced
this pull request
May 3, 2024
* origin/main: make service url gate friendlier (#3841) remove broken keyboard offset code (#3842) [Clipclops] Header tweaks (#3839) update date logic to account for timezones (#3840) [Clipclops] Moar error (#3837) [Session] Reset global agent on expire (#3838) [Session] Rely on agent session change event for persisting resumed session (#3836)
This was referenced May 3, 2024
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes a mistake I introduced in #3836.
Extracted from #3728.
The change in #3836 assumes the persistence handler will handle any errors. However, our
onAgentSessionChangepersistence handler does not currently update__globalAgenton expiry (despite updating the related state). This is inconsistent with how the same persistence handler treatsnetwork-error(for which it does update__globalAgent).Updating the global agent seems like the correct behavior if we're updating the UI to show PWI. So this seems like an omission in the existing code (which #3836 made worse). I suppose this was less important before #3836 because the
catchbranch would restore__globalAgentregardless of the persistence handler. Anyway, this fix is in the handler itself.In this PR, I adjust the
expiredbranch to do the same things (copy paste) as thenetwork-errorearly condition. (Unlike withnetwork-error, we don't want an early return because we still want theaccounts-related state update below to run.)Test Plan
To trigger the
expirycodepath, you can set a breakpoint inresumeSessionand evaluateres.data.did = 'foo'so that it raises an error which the agent will broadcast as an expiry.Verify that previously
__globalAgentwas being set in this case by thecatchblock I removed in #3836. So now it's not being set at all, and you can verify it's stale.After applying this PR and doing the same thing, verify that
__globalAgentis being set by the persisted handler.Verify you get kicked to the login and can log in after entering credentials.