ably / ably-js

Javascript, Node, Typescript, React, React Native client library SDK for Ably realtime messaging service
https://ably.com/download
Apache License 2.0
316 stars 55 forks source link

"Error: Connection closed" even when connection is open #1565

Open TranquilMarmot opened 10 months ago

TranquilMarmot commented 10 months ago

In our Next.js app, we're creating / closing an Ably realtime client inside of a useEffect.

const createAblyRealtimeClient = (authParams?: Record<string, string>) => {
  return new Realtime({
    authUrl: AUTH_URL,
    authMethod: "POST",
    authParams,
  });
};

export const useAblyRealtimeClient = (authParams?: Record<string, string>) => {
  const clientRef = useRef<Realtime | null>(null);

  useEffect(() => {
    // don't create the ably client on the server because it will cause errors
    if (typeof window !== "undefined") {
      clientRef.current = createAblyRealtimeClient(authParams);
    }

    return () => {
      // close the Ably connection when the component unmounts
      if (
        clientRef.current &&
        ["initialized", "connected", "connecting"].includes(
          clientRef.current?.connection.state,
        )
      ) {
        try {
          clientRef.current?.close();
        } catch (error) {
          console.error("Error closing Ably client", error);
        }
      }

      clientRef.current = null;
    };
  }, [authParams?.chatId]);

  return clientRef.current;
};

This client is then passed as a prop to an AblyProvider.

However, this still causes the Ably client to throw an error. Next.js will show us this: image

More of a stack trace...

Uncaught (in promise) Error: Connection closed
    at new ErrorInfo (webpack-internal:///(app-pages-browser)/../../node_modules/.pnpm/ably@1.2.48_react-dom@18.2.0_react@18.2.0/node_modules/ably/build/ably-commonjs.js:1154:28)
    at ErrorInfo.fromValues (webpack-internal:///(app-pages-browser)/../../node_modules/.pnpm/ably@1.2.48_react-dom@18.2.0_react@18.2.0/node_modules/ably/build/ably-commonjs.js:1171:36)
    at Object.closed (webpack-internal:///(app-pages-browser)/../../node_modules/.pnpm/ably@1.2.48_react-dom@18.2.0_react@18.2.0/node_modules/ably/build/ably-commonjs.js:5172:36)
    at Transport.close (webpack-internal:///(app-pages-browser)/../../node_modules/.pnpm/ably@1.2.48_react-dom@18.2.0_react@18.2.0/node_modules/ably/build/ably-commonjs.js:5513:58)
    at ConnectionManager.closeImpl (webpack-internal:///(app-pages-browser)/../../node_modules/.pnpm/ably@1.2.48_react-dom@18.2.0_react@18.2.0/node_modules/ably/build/ably-commonjs.js:8942:48)
    at ConnectionManager.requestState (webpack-internal:///(app-pages-browser)/../../node_modules/.pnpm/ably@1.2.48_react-dom@18.2.0_react@18.2.0/node_modules/ably/build/ably-commonjs.js:8693:18)
    at Connection.close (webpack-internal:///(app-pages-browser)/../../node_modules/.pnpm/ably@1.2.48_react-dom@18.2.0_react@18.2.0/node_modules/ably/build/ably-commonjs.js:12046:32)
    at Realtime.close (webpack-internal:///(app-pages-browser)/../../node_modules/.pnpm/ably@1.2.48_react-dom@18.2.0_react@18.2.0/node_modules/ably/build/ably-commonjs.js:11853:25)
    at eval (webpack-internal:///(app-pages-browser)/./src/lib/ably-client.ts:53:137)

Some questions...

What status are we supposed to check for when closing the connection? If I log clientRef.current?.connection.state before we call clientRef.current?.close(); it's shown as connecting or connected.

Why can't we catch this error? Even with the close() call inside of a try/catch, it isn't being caught (maybe this is an issue inside Next.js?)

┆Issue is synchronized with this Jira Bug by Unito

sync-by-unito[bot] commented 10 months ago

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-4019

TranquilMarmot commented 9 months ago

It seems like this error might be only caused in React dev mode since it calls hooks twice. Still curious about why we can't catch the error 🤔

owenpearson commented 9 months ago

Hey again @TranquilMarmot,

Apologies for the confusion here - the reason try catch isn't working is because client.close isn't actually throwing the exception. The "Connection closed" error is a generic error we throw when you call certain realtime methods on a closed client (for example, if you try to attach to a channel when the connection is closed it will throw a "Connection closed" error). Currently this error is created when the connection is closed and a reference to the error stored for use later, but since javascript error stack traces point to where the Error was constructed, this means that the stack trace will point to where you called client.close in your app, rather than where the error was actually thrown. I realise that this is quite rubbish for dev-experience so I've created https://github.com/ably/ably-js/issues/1573 to amend this behaviour.

For now, until we've fixed that issue, I think you probably would have to manually debug where the error is being thrown from I'm afraid. From the looks of it, it would seem to me that in react strict mode your useAblyRealtimeClient cleanup function would execute twice since strict mode will immediately unmount and re-mount any component which uses it, so this would result in the connection being closed immediately.

TranquilMarmot commented 9 months ago

Error occurs inside a Promise, as seen in the stack trace you provided "Uncaught (in promise) Error:"

Does that mean that the error is happening inside of a promise in RealtimeBase.close():? It seems like there's no way for us to catch it outside of the library.

It would be nice if close returned a Promise that we could await so that we know when it's actually closed.

TranquilMarmot commented 9 months ago

I think we may have gotten around this by tracking when we've already closed the connection in the hook:

export const useAblyRealtimeClient = (authParams?: Record<string, string>) => {
  const clientRef = useRef<Realtime | null>(null);
  const hasClosed = useRef(false);

  useEffect(() => {
    // don't create the ably client on the server because it will cause errors
    if (typeof window !== "undefined") {
      clientRef.current = createAblyRealtimeClient(authParams);
      hasClosed.current = false;
    }

    return () => {
      if (
        !hasClosed.current &&
        clientRef.current &&
        ["initialized", "connected", "connecting"].includes(
          clientRef.current?.connection.state,
        )
      ) {
        clientRef.current?.close();
        hasClosed.current = true;
      }

      clientRef.current = null;
    };
  }, [authParams?.chatId]);

  return clientRef.current;
};
nikitadubyk commented 6 months ago

I have the same issue only in dev mode, no such error on stage

nikitadubyk commented 6 months ago

@VeskeR @owenpearson I also noticed that this error occurs after unmout in ablyClient close method. During the call of this method the error Error: Connection closed appears.

  const [ablyClient, setAblyClient] = useState<Realtime | undefined>(undefined);
  const [ablyChannel, setAblyChannel] = useState<RealtimeChannel | undefined>(
    undefined
  );

  const destroyAbly = () => {
    setAblyError(undefined);
    if (ablyChannel) {
      ablyChannel.off();
      setAblyChannel(undefined);
    }
    if (ablyClient) {
      ablyClient.close(); // Here error after call close
      setAblyClient(undefined);
    }
    setAblyConnected(false);
  };

Example of how destroyAbly is used during unmount

  const destoryRef = useRef(destroyAbly);

  useEffect(() => {
    destoryRef.current = destroyAbly;
  }, [destroyAbly]);

    useEffect(() => {
    return () => {
      destoryRef.current?.();
    };
  }, []);

I am using the latest version of ably - 2.0.3