[CLNP-8706] getchannel retry on reconnect#1441
Open
sf-tyler-jeong wants to merge 4 commits into
Open
Conversation
GroupChannelProvider fetched the channel via getChannel(channelUrl) in a useAsyncEffect keyed only on [sdkStore.initialized, sdkStore.sdk, channelUrl]. When that fetch was cancelled or failed during an SDK disconnect->reconnect cycle, handleChannelError set currentChannel to null and nothing re-ran the effect on a bare auto-reconnect (none of those deps change), so the conversation stayed stuck on the invalid / 'channel is required' state until a manual remount. Add config.isOnline to the effect's dependency array so the fetch retries once the SDK reconnects (config.isOnline is driven by onReconnectSucceeded), recovering a channel that previously stayed null. Skip the fetch when we already hold this channel loaded without error: this avoids a redundant refetch on healthy reconnects and avoids nulling out a healthy channel when the connection drops. Because isOnline in the deps can re-run the effect while a getChannel is still in flight, track a request id and ignore stale resolutions so a late resolve or reject cannot clobber a newer fetch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Regression tests for the reconnect-retry fix: (1) a getChannel that fails during a disconnect is retried and recovers currentChannel once config.isOnline flips false->true; (2) an already-loaded channel is not refetched across a disconnect/reconnect cycle; (3) a stale getChannel rejection arriving after a newer fetch has succeeded is ignored by the request-id guard and does not clobber the recovered channel. Uses stable sdk/config mock refs so only config.isOnline changes drive the effect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for sendbird-uikit-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c29ba495e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Follow-up to the reconnect-retry guard. The skip-when-already-loaded guard also suppressed the refetch when SendbirdProvider replaces sdkStore.sdk for a new appId/userId session: GroupChannelProvider is keyed only by channelUrl, so a same-URL currentChannel from the previous SDK survived and the effect returned early, leaving the UI/message actions bound to the previous SDK's channel object. Track the SDK instance the channel was fetched with and only skip when it still matches sdkStore.sdk, so a new SDK instance triggers a refetch. Adds a regression test for the SDK-swap case. Addresses the Codex review comment on PR #1441. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@chatgpt-codex-connector review again |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
danney-chun
reviewed
Jul 6, 2026
| // failed during a disconnect would leave currentChannel === null with no retry. | ||
| const getChannelRequestIdRef = useRef(0); | ||
| const fetchedChannelSdkRef = useRef<unknown>(null); | ||
| useAsyncEffect(async () => { |
Contributor
There was a problem hiding this comment.
issue에 대한 문제는 처리가 되는 것으로 보입니다.
다만, config.isOnline이 deps에 추가되는 시점에 useAsyncEffect()를 사용하는 것이 아닌, useEffect()를 사용하여 cleanUp을 활용하는 것이 어떨까 하는 생각입니다.
Suggested change
| useAsyncEffect(async () => { | |
| const fetchedChannelSdkRef = useRef<unknown>(null); | |
| useEffect(() => { | |
| if (!sdkStore.initialized || !channelUrl) return; | |
| if (state.currentChannel?.url === channelUrl | |
| && !state.fetchedChannelError | |
| && fetchedChannelSdkRef.current === sdkstore.sdk ) return; | |
| let candelled = false; | |
| (async () => { | |
| try { | |
| const channel = await sdkStore.sdk.groupChannel.getChannel(channelUrl); | |
| if (cancelled) return; | |
| actions.setCurrentChannel(channel); | |
| } catch(error) { | |
| logger?.error?.('GroupChannelProvider: error when fetching channel', error); | |
| if (cancelled) return; | |
| actions.handleChannelError(error); | |
| } | |
| })(); | |
| return () => { cancelled = true; }; | |
| }, [sdkStore.initialized, sdkStore.sdk, channelUrl, config.isOnline]); |
이런 형태를 생각해 봤습니다.
Contributor
Author
There was a problem hiding this comment.
수정했습니다. 감사합니다!
Per review feedback on PR #1441 (danney-chun), replace the useAsyncEffect + request-id counter with a plain useEffect whose cleanup sets a 'cancelled' flag, so a superseded/in-flight getChannel cannot clobber a newer fetch (and it also no-ops after unmount). Behavior is unchanged; the SDK-instance skip-guard (fetchedChannelSdkRef) is kept. Drops the now-unused useAsyncEffect import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
GroupChannelProviderfetches its channel viagetChannel(channelUrl)in auseAsyncEffect. If that fetch was cancelled or failed during an SDKdisconnect→reconnect cycle,
currentChannelstayednulland the effect neverretried on a bare auto-reconnect (its deps didn't include connection state), so
the conversation view got stuck and
useGroupChannelMessageslogged[useGroupChannelMessages] channel is required. The Desk ticket/channel stillexisted server-side. Reported on
@sendbird/uikit-react@3.16.9/@sendbird/chat@4.19.1; reproduced on currentmain.Root cause
The channel-init effect's deps were
[sdkStore.initialized, sdkStore.sdk, channelUrl],none of which change on an auto-reconnect, so a failed
getChannelwas never retried.Changes
src/modules/GroupChannel/context/GroupChannelProvider.tsxconfig.isOnlineto the effect deps so the fetch retries once the SDKreconnects (
isOnlineis driven byonReconnectSucceeded).redundant refetch on healthy reconnects and avoids nulling out a healthy
channel when the connection drops.
getChannelresolve/reject can't clobber anewer fetch (the reconnect-driven re-runs make overlapping requests possible).
Tests
src/modules/GroupChannel/context/__tests__/GroupChannelProvider.reconnect.spec.tsx(new)isOnlinefalse→true → retry succeeds →currentChannelrecoversBackward compatibility
No public API / prop / export / type / StringSet changes. Offline behavior with
local cache is unchanged (
getChannelstill serves from the local cache).Notes
The ticket also suggested null-guarding
useGroupChannelMessagesinstead of thecurrentChannel!non-null assertion. That hook lives in@sendbird/uikit-toolsand is already runtime null-safe (init no-ops while the channel is null), so it
isn't required for recovery and is out of scope here.