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
313 stars 55 forks source link

Avoiding `clientId mismatch` error when user's session up/downgrades #1600

Open smblee opened 7 months ago

smblee commented 7 months ago

Currently when user's clientId changes (e.g. logs in), clientId will go from undefined to "my-user-id", which I do with

const { user } = useUser();
const { ably } = useAbly();
useEffect(() => {
  ably.auth.authorize();
}, [user])

Our custom API endpoint ably calls to get the token will take care of figuring out the clientId/capabilities based on the cookie session.

After the request, however, any subsequent realtime connections break with "clientId mismatch" error.

I would like to know if there's a way to cleanly cleanup and restart the Ably auth context to avoid this error

┆Issue is synchronized with this Jira Story by Unito

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

➤ Automation for Jira commented:

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

bookercodes commented 7 months ago

Hey @smblee, thanks for sharing. I shared this internally to bring some attention to it.

As I mentioned on X, I hope someone knows the answer quick.

If not, they may need to triage and prioritise it relevant to other issues.

ttypic commented 7 months ago

Hi @smblee,

Unfortunately, once a connection is established, it's not possible to change the associated clientId. So, the simplest solution is to create a new connection.

Could you please share your use case? How do you use Ably for unauthorized clients?

smblee commented 7 months ago

Say this is the structure

<RealTimeProvider>
  <UserProvider>
   ...
   {children}
  </ UserProvider>
</ RealtimeProvider>
const RealTimeProvider = ({ children }: { children: React.ReactNode }) => {
  const client = new Ably.Realtime.Promise({
    authUrl: "/api/ably-token",
    authMethod: "POST"
  });

  return (
    <AblyProvider client={client}>
      <_RealTimeProvider>{children}</_RealTimeProvider>
    </AblyProvider>
  );
};

the /api/ably-token returns clientId of either undefined (unset) or the user's id, based on the user session cookie (and custom capabilities)

And imagine there the UserProvider contains data on the user that's either User | null (depending on session). If signed in, user exists, if not null.

const UserProvider = () => {
  const { user } = ...;
  const  ably = useAbly();

  useEffect(() => {
    ably.auth.authorize(); // after authorizing, any subsequent `useChannel()` hooks will start failing with `mismatched clientId` error
  }, [user, ably])

This is my current setup.

thenano commented 7 months ago

I have the exact same issue with the same type of setup, and I'm getting stuck in the same situation. I'll explain my use case so that perhaps it sheds some light on how this scenario comes about. My application uses ably for realtime notifications for anyone in the app, including non-logged in users. These users still authenticate with my app via the authUrl strategy and my server returns a tokenRequest without a clientId (since these users are not authenticated) with subscribe capabilities. For logged in users, we return a tokenRequest with client id and with the additional capability for presence in certain channels. Everything works fine except when a user logs in during their session, and so they "upgrade" from a connection without clientId to one with clientId, and then I receive the same errors posted in this issue after we explicitly call authorize. I have also tried closing the existing connection and creating a new one (by just creating a completely new Ably.Realtime instance). This prevents the issue described here, but I essentially lose all existing subscriptions and can't seem to get the to reload properly. Hope this helps!

ttypic commented 7 months ago

@thenano @smblee thank you very much for sharing. We will be looking into how to improve the developer experience for this use case and will keep you posted.

In the meantime, @smblee, I suggest this workaround as a potential solution:

const RealTimeProvider = ({ children }: { children: React.ReactNode }) => {
  const [client, setClient] = useState(() => new Ably.Realtime.Promise({
    authUrl: "/api/ably-token",
    authMethod: "POST"
  }));

  const reloadAbly = useCallback(() => {
    setClient(prevClient => { 
      prevClient.close();
      return new Ably.Realtime.Promise({
         authUrl: "/api/ably-token",
         authMethod: "POST"
      });
    });
  }, []);

  return (
    <AblyProvider client={client}>
      {/* use existing or new `React.Context` to pass `reloadAbly` through */}
      <_RealTimeProvider reloadAbly={reloadAbly}>{children}</_RealTimeProvider>
    </AblyProvider>
  );
};
ttypic commented 7 months ago

@thenano,

but I essentially lose all existing subscriptions and can't seem to get the to reload properly.

Could you please clarify what it means when you mention losing all existing subscriptions and how you would like them to be reloaded?

smblee commented 7 months ago

CleanShot 2024-01-31 at 10 08 25@2x

Does this code make sense? I need to still make a new auth call if the user object changes for some reason (capabilities may change)

ttypic commented 7 months ago

Hi, @smblee. Yes, it does, if clientId hasn't been changed

thenano commented 7 months ago

Hi @ttypic my bad, it seems to work as expected, when I log in the hooks will re-render and subscriptions will re-establish. I did have another issue, just wanted to clarify implementation here: If I do this (specifically if I use Realtime.Promise)

const reloadAbly = useCallback(() => {
    setClient(prevClient => { 
      prevClient.close();
      return new Ably.Realtime.Promise({
         authUrl: "/api/ably-token",
         authMethod: "POST"
      });
    });
  }, []);

I get errors from prevClient.close(), like this:

Uncaught (in promise) Error: Connection closed
    at new ErrorInfo (ably-commonjs.js:1158:1)
    at ErrorInfo.fromValues (ably-commonjs.js:1175:1)
    at Object.closed (ably-commonjs.js:5176:1)
    at Transport.close (ably-commonjs.js:5517:1)
    at ConnectionManager.closeImpl (ably-commonjs.js:8946:1)
    at ConnectionManager.requestState (ably-commonjs.js:8697:1)
    at Connection.close (ably-commonjs.js:12050:1)
    at Realtime.close (ably-commonjs.js:11857:1)

These errors happen when I use usePresence, and specifically when the component using that hook unmounts, the error comes from leaving the channel, done on this line.. It's weird that the channel.state is still "attached", but I guess that means it's not updated it's state yet. Any ideas on what can be done to not get this error? I'm trying to just use the provided hooks and not have to implement the same functionality. As a note, this does not happen if I do not use Realtime.Promise and just use Realtime instead, but I have to use Realtime.Promise to get presenceData updates via the usePresence hook.

Another thing I'm seeing is that when I close the connection I also get a bunch of errors printed to the console:

 Unchecked runtime.lastError: The message port closed before a response was received.

and

 Unchecked runtime.lastError: Could not establish connection. Receiving end does not exist.

I'm wondering if I need to do anything differently? Or if there's a way to cleanly close the connection avoiding all these errors?

thanks!

ttypic commented 7 months ago

@thenano Thanks for reporting this! It looks like a bug, and we will fix it. As a workaround for now, you can add a delay for closing, for example:

const reloadAbly = useCallback(() => {
    setClient(prevClient => { 
      setTimeout(() => prevClient.close(), 1000);
      return new Ably.Realtime.Promise({
         authUrl: "/api/ably-token",
         authMethod: "POST"
      });
    });
  }, []);
ttypic commented 7 months ago

Hey @thenano,

We fixed this usePresence bug in 1.2.49

thenano commented 7 months ago

Hi @ttypic awesome thank you! I did see the note on the changelog. I haven't been able to validate because in the end there were some other issues with using Realtime.Promise so I ended up using the non-promise one, only downside really being the presenceData not updating with the presence hook.

thenano commented 2 weeks ago

Hi @ttypic, I'm trying to upgrade to v2 and I'm now getting a similar error to the one above when calling prevClient.close(), I'm wondering if the fix was ported to v2 as well? Let me know if you need any more information.

ttypic commented 1 week ago

Hey @thenano,

Thank you very much for reporting! Yes, fix was ported, could you please share verbose logs or reproducible example with us?

thenano commented 4 days ago

Hi @ttypic thanks for the followup! I'm now going to focus on this and I'm currently debugging it. The error is not actually consistent on my side which makes me feel like perhaps I'm the one doing something wrong. I'll update later when I have a better idea of what's happening.