bug: ChannelInner can call methods on a disconnected channel, causing uncaught errors

[wchargin]

Describe the bug

Repeatedly, quickly mount and unmount a component that instantiates a <Chat /> and connects on mount, then disconnects on unmount. Most of the time, this works fine. About 1–2% of the time, an unhandled error "You can't use a channel after client.disconnect() was called" is thrown, and the chat window no longer loads properly.

To Reproduce

See description above. I don't have a fully self-contained repro. I can share the structure of our code (below), but the main evidence that I have is the following stack trace:

Error:
    <anonymous> debugger eval code:1
    _callee32$ browser.es.js:3269
    Babel 11
    query browser.es.js:3303
    _callee28$ browser.es.js:2920
    Babel 9
    watch browser.es.js:2943
    getChannel getChannel.js:42
    step tslib.es6.mjs:147
    verb tslib.es6.mjs:128
    __awaiter tslib.es6.mjs:121
    __awaiter tslib.es6.mjs:117
    getChannel getChannel.js:18
    ChannelInner Channel.js:234
    step tslib.es6.mjs:147
    verb tslib.es6.mjs:128
    __awaiter tslib.es6.mjs:121
    __awaiter tslib.es6.mjs:117
    ChannelInner Channel.js:207
    ChannelInner Channel.js:277
    React 9
    workLoop scheduler.development.js:266
    flushWork scheduler.development.js:239
    performWorkUntilDeadline scheduler.development.js:533

I captured this stack trace with my debugger paused on the channel.ts error site, "You can't use a channel after client.disconnect() was called". The line Channel.js:207 corresponds to somewhere in this Babel-compiled code…

$ nl node_modules/stream-chat-react/dist/components/Channel/Channel.js | sed -n 203,210p
   203      // useLayoutEffect here to prevent spinner. Use Suspense when it is available in stable release
   204      useLayoutEffect(function () {
   205          var errored = false;
   206          var done = false;
   207          (function () { return __awaiter(void 0, void 0, void 0, function () {
   208              var members, _i, _a, member, userId, _b, user, user_id, config, e_2, _c, user, ownReadState;
   209              var _d, _e, _f, _g;
   210              return __generator(this, function (_h) {

…which, from the getChannel call, must be this line:

https://github.com/GetStream/stream-chat-react/blob/v11.8.0/src/components/Channel/Channel.tsx#L566

I don't see a clean fix that my code could employ to avoid this, since we're always passing in the same client, and the point where we're disconnecting is on unmount so we don't have the ability to do much else. We work around it for now by delaying 5000ms between unmounting and actually disconnecting, but this is undesirable for lots of obvious reasons. I think that stream-chat-react should guard its calls to channel/client methods to ensure that it does not call any when the client might be disconnected.

Sample code structure

export const Main: React.FC<Props> = ({ channelName, className, user, token }) => {
  const { current: waitGroup } = useRef(new WaitGroup());

  const [streamClient, setStreamClient] = useState<StreamChat | undefined>();
  useEffect(() => {
    // connect websocket once and only once
    let streamClient: StreamChat;
    const ac = new AbortController();
    (async () => {
      try {
        streamClient = new StreamChat(STREAM_API_KEY);
        await streamClient.connectUser({ id: token.userId, name: user.name }, token.token);
        if (ac.signal.aborted) {
          return;
        }
        setStreamClient(streamClient);
      } catch (error: any) {
        if ("StatusCode" in error) {
          console.error(String((error as APIErrorResponse).StatusCode));
        }
      }
    })();

    // disconnect websocket on unmount
    return () => {
      ac.abort();
      (async () => {
        await sleepMs(5 * 1000);  // XXX: needed to work around this issue!
        await waitGroup.wait(10 * 1000);
        streamClient?.disconnectUser();
      })();
    };
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  const channel = useMemo(() => {
    if (!streamClient?.user) {
      return;
    }
    return streamClient.channel("team", channelName, {
      name: channelName,
    });
  }, [channelName, t, streamClient]);

  useEffect(() => {
    if (channel) {
      waitGroup.add(channel.watch());
      return () => {
        if (!channel.disconnected) {
          waitGroup.add(channel.stopWatching());
        }
      };
    }
  }, [channel, waitGroup /* constant */]);

  if (!streamClient || streamClient.wsConnection?.isConnecting) {
    return "Loading...";
  }

  return (
    <Chat client={streamClient}>
      <Channel
        channel={channel}
        reactionOptions={OUR_REACTION_OPTIONS}
        UnreadMessagesNotification={UnreadMessagesNotification}
        UnreadMessagesSeparator={UnreadMessagesSeparator}
      >
        <Window>
          <VirtualizedMessageList
            Message={OurMessageComponent}
            shouldGroupByUser
          />
          <MessageInput grow />
        </Window>
        <Thread Message={OurMessageComponent} />
      </Channel>
    </Chat>
  );
};

class WaitGroup {
  constructor() {
    this._pending = new Set();
  }

  add<T>(promise: Promise<T>): Promise<T> {
    promise = Promise.resolve(promise); // defensive, in case of type-corrupted non-promise value
    const ref = new WeakRef(promise);
    this._pending.add(ref);
    return promise.finally(() => this._pending.delete(ref));
  }

  async wait(): Promise<void> {
    // Defer a frame so that any promises that are added to the wait group this
    // event loop get properly awaited. Particularly useful since execution
    // order of React `useEffect` cleanup handlers is undefined by design:
    // https://github.com/facebook/react/issues/16728#issuecomment-584208473
    await sleepMs(0);

    const pending = Array.from(this._pending, (weakRef) => weakRef.deref());
    await Promise.race([sleepMs(timeoutMs), Promise.allSettled(pending)]);
  }
}

Expected behavior

A rendered <Channel channel={channel} /> should never throw an internal error due to using a closed client.

Screenshots

Package version

• stream-chat-react: v11.8.0
• stream-chat-css: N/A? (not in my package-lock.json)
• stream-chat-js: v8.16.0

Desktop (please complete the following information):

• OS: macOS Sonoma 14.3
• Browser: Firefox
• Version: 126.0

Smartphone (please complete the following information):

• Device: N/A
• OS: N/A
• Browser: N/A
• Version: N/A

Additional context

N/A

[arnautov-anton]

Hey, @wchargin,

this is a known set of somewhat big longstanding issues that have yet to be resolved. As one of the fixes we've introduced useCreateChatClient hook (see Getting Started), which handles instantiation and connection handling for you. It's not recommended to reuse the same client instance for different users as that comes with its own set of issues, the hook takes care of this problem by creating new instance each time the user.id changes. If you want/need to switch between users fast and want to avoid initiating connections, see this unreleased implementation which delays initiating connection for a specified amount of time.

Thank you for submitting and let me know if you encounter simillar issue with the suggested solution.

[wchargin]

Hey @arnautov-anton; thanks for your quick response. I took a look at useCreateChatClient.ts but I don't see how we could fit it into our use case. If we were to use that hook and then watch a channel—

const chatClient = useCreateChatClient();

const channel = useMemo(() => {
  if (streamClient) {
    return streamClient.channel("team", "some-channel-id");
  }
}, [chatClient]);

useEffect(() => {
  if (channel) {
    channel.watch();
    return () => {
      channel.stopWatching();
    };
  }
}, [channel]);

—then how could we ensure that the client does not get disconnected while the watch or stopWatching API calls are in progress? Really, this applies if we make any stream API call that depends on the client being open until it returns. Since the client is closed outside of our control, I don't see how we could avoid getting the "You can't use a channel after client.disconnect() was called" errors.

(We're not trying to switch between users quickly or at all; it's just one user per session, and we do not expect the user ID to ever change. This issue occurs when you simply unmount the component by navigating away to a different part of the client-side application.)

[arnautov-anton]

Ah, I see - yes, this might be a bit problematic as our getClient method (used by almost every other method within Channel class) checks for disconnected status and throws if the client has been disconnected. Here's an adjusted implementation which sets channel only after watch has been called (and it has not been interrupted by cleanup), we can skip calling stopWatching if the instance is already disconnected:

const [channel, setChannel] = useState<Channel | undefined>(undefined);

useEffect(() => {
  if (!chatClient) return;

  let interrupted = false;

  const c = chatClient.channel('team', 'some-channel-id');

  c.watch().then(() => {
    if (!interrupted) setChannel(c);
  });

  return () => {
    setChannel(undefined);
    interrupted = true;
    if (c.disconnected) return;
    c.stopWatching();
  };
}, [chatClient]);

I did some local testing trying to rapidly mount/unmount the whole chat component tree and while the amount of thrown errors has decreased, it's still not 0 and even with this approach I still sometimes get the error you mentioned. It's better than passing uninitialized channel instance directly to Channel component but still not ideal.

This issue - or rather - set of issues is pretty well known to us and is on our list but it's currently not prioritized. We'll update this thread once there's any progress.

[mkusold]

Curious if there's been any progress on this issue.