Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getToken method returns null right after isSIgnedIn becomes true #2266

Closed
4 tasks done
kylegwalsh opened this issue Dec 5, 2023 · 7 comments
Closed
4 tasks done

getToken method returns null right after isSIgnedIn becomes true #2266

kylegwalsh opened this issue Dec 5, 2023 · 7 comments

Comments

@kylegwalsh
Copy link

Preliminary Checks

Reproduction / Replay Link

https://jam.dev/c/3bce6734-c313-4b82-a638-4022254bfb7a

Publishable key

pk_test_Y2xlcmsuY3Jpc3Auc2Vhc25haWwtOTEubGNsLmRldiQ

Description

Steps to reproduce:

  1. Log in using the useSignIn() hook
  2. Enable queries and call getToken() from the useAuth() hook once the user logs in (key off of the isSignedIn boolean)

Expected behavior:

I would expect getToken() to return the token the moment clerk indicates that isSignedIn is true. This is how the library behaved in version 0.10.6 of clerk-expo (and it's related clerk-react version).

Actual behavior:

getToken() returns null right after the user is signed in (a race condition). This causes a cascade of undesired behaviors in my particular app because I expect token to exist right after clerk informs me that the user isSignedIn.

Environment

System:
    OS: macOS 13.3.1
    CPU: (8) arm64 Apple M1
    Memory: 434.08 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 16.17.0 - ~/.nvm/versions/node/v16.17.0/bin/node
    Yarn: 3.2.2 - /opt/homebrew/bin/yarn
    npm: 8.15.0 - ~/.nvm/versions/node/v16.17.0/bin/npm
    pnpm: 8.7.4 - /opt/homebrew/bin/pnpm
  Browsers:
    Chrome: 119.0.6045.199
    Safari: 16.4
  npmPackages:
    @babel/core: ^7.20.0 => 7.22.1 
    @babel/plugin-proposal-export-namespace-from: 7.18.9 => 7.18.9 
    @clerk/clerk-expo: 0.19.22 => 0.19.22 
    @expo/vector-icons: ^13.0.0 => 13.0.0 
    @floating-ui/react-native: 0.8.2 => 0.8.2 
    @formatjs/intl-locale: 2.4.41 => 2.4.41 
    @formatjs/intl-numberformat: 7.3.0 => 7.3.0 
    @formatjs/intl-pluralrules: 4.2.0 => 4.2.0 
    @gorhom/bottom-sheet: 4.4.5 => 4.4.5 
    @hlp/analytics: 1.0.0 => 1.0.0 
    @hlp/assets: 1.0.0 => 1.0.0 
    @hlp/backend: 1.0.0 => 1.0.0 
    @hlp/components: 1.0.0 => 1.0.0 
    @hlp/config: 1.0.0 => 1.0.0 
    @hlp/core: 1.0.0 => 1.0.0 
    @hlp/expo-plugins: 1.0.0 => 1.0.0 
    @hlp/hooks: 1.0.0 => 1.0.0 
    @hlp/theme: 1.0.0 => 1.0.0 
    @hookform/resolvers: 2.9.7 => 2.9.7 
    @react-native-aria/checkbox: 0.2.3 => 0.2.3 
    @react-native-async-storage/async-storage: 1.17.11 => 1.17.11 
    @react-native-community/netinfo: 9.3.7 => 9.3.7 
    @react-navigation/bottom-tabs: 6.3.2 => 6.3.2 
    @react-navigation/devtools: 6.0.8 => 6.0.8 
    @react-navigation/drawer: 6.5.0 => 6.5.0 
    @react-navigation/native: 6.0.8 => 6.0.8 
    @react-navigation/native-stack: 6.5.0 => 6.5.0 
    @react-navigation/stack: 6.2.2 => 6.2.2 
    @react-stately/checkbox: 3.3.1 => 3.3.1 
    @rnmapbox/maps: 10.0.0-beta.58 => 10.0.0-beta.58 
    @segment/analytics-react-native: 2.7.1 => 2.7.1 
    @segment/sovran-react-native: 0.4.4 => 0.4.4 
    @sentry/react-native: 4.13.0 => 4.13.0 
    @shopify/flash-list: 1.4.0 => 1.4.0 
    @shopify/react-native-skia: 0.1.190 => 0.1.190 
    @tanstack/react-query: 4.2.3 => 4.2.3 
    @trpc/client: 10.0.0-proxy-beta.19 => 10.0.0-proxy-beta.19 
    @trpc/react: 10.0.0-proxy-beta.19 => 10.0.0-proxy-beta.19 
    @types/aws-lambda: 8.10.101 => 8.10.101 
    @types/chroma-js: 2 => 2.4.0 
    @types/number-to-words: 1 => 1.2.1 
    @types/numeral: ^2 => 2.0.5 
    @types/react-helmet: 6 => 6.1.6 
    @types/react-native-typing-animation: 0.1.0 => 0.1.0 
    asyncstorage-down: 4.2.0 => 4.2.0 
    babel-plugin-module-resolver: 4.1.0 => 4.1.0 
    chroma-js: 2.4.2 => 2.4.2 
    date-fns: 2.29.3 => 2.29.3 
    eslint: 8.27.0 => 8.27.0 
    expo: ~48.0.21 => 48.0.21 
    expo-application: ~5.1.1 => 5.1.1 
    expo-asset: ~8.9.1 => 8.9.2 
    expo-auth-session: ~4.0.3 => 4.0.3 
    expo-av: ~13.2.1 => 13.2.1 
    expo-build-properties: ~0.6.0 => 0.6.0 
    expo-constants: ~14.2.1 => 14.2.1 
    expo-dev-client: ~2.2.1 => 2.2.1 
    expo-device: ~5.2.1 => 5.2.1 
    expo-document-picker: ~11.2.2 => 11.2.2 
    expo-file-system: ~15.2.2 => 15.2.2 
    expo-linear-gradient: ~12.1.2 => 12.1.2 
    expo-linking: ~4.0.1 => 4.0.1 
    expo-local-authentication: ~13.3.0 => 13.3.0 
    expo-localization: ~14.1.1 => 14.1.1 
    expo-location: ~15.1.1 => 15.1.1 
    expo-modules-core: ~1.2.7 => 1.2.7 
    expo-navigation-bar: ~2.1.1 => 2.1.1 
    expo-notifications: ~0.18.1 => 0.18.1 
    expo-router: 0.0.36 => 0.0.36 
    expo-secure-store: ~12.1.1 => 12.1.1 
    expo-sharing: ~11.2.2 => 11.2.2 
    expo-splash-screen: ~0.18.2 => 0.18.2 
    expo-status-bar: ~1.4.4 => 1.4.4 
    expo-store-review: ~6.2.1 => 6.2.1 
    expo-system-ui: ~2.2.1 => 2.2.1 
    expo-updates: ~0.16.4 => 0.16.4 
    expo-web-browser: ~12.1.1 => 12.1.1 
    expo-yarn-workspaces: 2.0.2 => 2.0.2 
    lottie-react-native: 5.1.4 => 5.1.4 
    moti: 0.20.0 => 0.20.0 
    native-base: v3.4.28 => 3.4.28 
    node-libs-react-native: 1.2.1 => 1.2.1 
    number-to-words: 1.2.4 => 1.2.4 
    numeral: 2.0.6 => 2.0.6 
    posthog-react-native: 2.1.0 => 2.1.0 
    react: 18.2.0 => 18.1.0 
    react-date-picker: 9.0.0 => 9.0.0 
    react-dom: 18.2.0 => 18.1.0 
    react-error-overlay: 6.0.9 => 6.0.9 
    react-helmet: 6.1.0 => 6.1.0 
    react-map-gl: 7.0.19 => 7.0.19 
    react-native: 0.71.14 => 0.71.14 
    react-native-appsflyer: 6.12.2 => 6.12.2 
    react-native-awesome-alerts: 2.0.0 => 2.0.0 
    react-native-bundle-visualizer: 3.1.1 => 3.1.1 
    react-native-confetti-cannon: 1.5.2 => 1.5.2 
    react-native-date-picker: 4.2.5 => 4.2.5 
    react-native-email-link: 1.14.3 => 1.14.3 
    react-native-fast-image: 8.5.11 => 8.5.11 
    react-native-gesture-handler: ~2.9.0 => 2.9.0 
    react-native-get-random-values: ~1.8.0 => 1.8.0 
    react-native-gifted-charts: 1.3.12 => 1.3.12 
    react-native-gifted-chat: 1.1.1 => 1.1.1 
    react-native-hyperlink: 0.0.22 => 0.0.22 
    react-native-linear-gradient: 2.8.3 => 2.8.3 
    react-native-lottie: 0.0.1-alpha.2 => 0.0.1-alpha.2 
    react-native-mask-input: 1.2.2 => 1.2.2 
    react-native-mmkv: 2.8.0 => 2.8.0 
    react-native-monorepo-tools: 1.2.0 => 1.2.0 
    react-native-plaid-link-sdk: 10.3.0 => 10.3.0 
    react-native-popper: 0.3.2 => 0.3.2 
    react-native-reanimated: 3.3.0 => 3.3.0 
    react-native-redash: 18.1.0 => 18.1.0 
    react-native-safe-area-context: 4.5.0 => 4.5.0 
    react-native-screens: ~3.20.0 => 3.20.0 
    react-native-svg: 13.14.0 => 13.14.0 
    react-native-swiper-flatlist: 3.0.17 => 3.0.17 
    react-native-toast-message: 2.1.5 => 2.1.5 
    react-native-typing-animation: 0.1.7 => 0.1.7 
    react-native-web: ~0.18.11 => 0.18.12 
    react-native-webview: 13.2.2 => 13.2.2 
    react-plaid-link: 3.4.0 => 3.4.0 
    react-select: 5.6.0 => 5.6.0 
    recharts: 2.9.0 => 2.9.0 
    rn-nodeify: 10.3.0 => 10.3.0 
    sentry-expo: ~6.1.0 => 6.1.1 
    smartlook-react-native-wrapper: 1.2.8 => 1.2.8 
    superjson: 1.9.1 => 1.9.1 
    ts-node: 10.9.1 => 10.9.1 
    zod: 3.20.2 => 3.20.2
@kylegwalsh kylegwalsh added the needs-triage A ticket that needs to be triaged by a team member label Dec 5, 2023
@jescalan jescalan added prioritized This issue has been triaged and the team is working on it and removed needs-triage A ticket that needs to be triaged by a team member labels Dec 11, 2023
@LekoArts LekoArts added confirmed and removed prioritized This issue has been triaged and the team is working on it labels Jan 29, 2024
@desiprisg
Copy link
Member

@kylegwalsh , Having seen the video and checked the behaviour myself to double check, getToken is returning a Promise, and thus it will have to be called after isSignedIn has returned true. As seen below, getToken() returned null before the log for isSignedIn is printed and so, it makes sense that it might returnnull, if isSignedIn is not checked beforehand.
I should also mention here, that one should not look at the sign-ins request in the network tab, but the touch request right after, as that is what is actually setting the session as the active one.
image

Since you mentioned that you actually check the value of isSignedIn before you actually call getToken() (although the logs in the console seem to be reversed), could you provide a snippet of the code ?

@kylegwalsh
Copy link
Author

Hi @desiprisg, my code uses @trpc/react to control queries to the backend. My query should not be enabled until isSignedIn becomes true (see below):

useUser() hook

  // Get the user data from our DB
  const { data } = trpc.user.get.useQuery(undefined, {
    // Don't enable the query until the clerk user is defined
    enabled: !!isSignedIn,
  });

QueryProvider component

import { useAuth } from '@clerk/clerk-expo';
import { config } from '@hlp/config';
import { trpc } from '@hlp/core/client';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { httpBatchLink } from '@trpc/client/links/httpBatchLink';
import React, { useState } from 'react';
import superjson from 'superjson';

/** Adds TRPC and react-query functionality to the app */
export const QueryProvider = ({ children }: any) => {
  const { getToken } = useAuth();

  // Initialize query client and trpc client
  const [queryClient] = useState(() => new QueryClient());
  const [trpcClient] = useState(() =>
    trpc.createClient({
      links: [
        httpBatchLink({
          url: `${config.api.url}/trpc/`,
          async headers() {
            /** Retrieve auth token from storage */
            const token = await getToken({
              template: 'default',
            });

            console.log('CLERK TOKEN', token);

            return {
              // Attach auth header if we have a token
              ...(token && {
                Authorization: `Bearer ${token}`,
              }),
            };
          },
        }),
      ],
      transformer: superjson,
    })
  );

  return (
    <trpc.Provider client={trpcClient} queryClient={queryClient}>
      <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
    </trpc.Provider>
  );
};

So, the useUser() query is being enabled by isSignedIn first, and then it is invoking getToken() during the request. This is why I thought that there may be some kind of race condition between the two actions.

I just attempted to upgrade to the latest clerk version to see if the issues still persist, but I'm seeing a CORS error due to an expo header (I never saw this before, so I'm not sure what introduced it). I'll have to dig into this more before I can determine if I'm still seeing the issue.

Access to fetch at 'https://CLERK_DOMAIN/v1/client?_clerk_js_version=4.70.5&_is_native=1&__dev_session=SESSION' from origin 'http://localhost:8081' has been blocked by CORS policy: Request header field x-expo-native-application-version is not allowed by Access-Control-Allow-Headers in preflight response.

@gkamer8
Copy link

gkamer8 commented Apr 8, 2024

We also have pages protected by whose useEffects are triggered while await getToken() returns null (same situation as above - directly after the user signs in and gets redirected). Is there active work on this issue / what is the best practice here to prevent a double fetch?

@thiskevinwang
Copy link
Contributor

Note

Those header(s) are coming from the Expo SDK.

// Send some non-identifying headers that are useful for logging
(requestInit.headers as Headers).set('x-expo-execution-environment', Constants.executionEnvironment);
(requestInit.headers as Headers).set(
'x-expo-native-application-version',
Application.nativeApplicationVersion || 'NULL',
);
});

We can remove these, and publish a new version.

Additionally, the CORS suggests that you're using Clerk-expo from a browser, is that right @kylegwalsh? Can you help us understand your development flow a bit? Clerk-expo via the browser is not exactly a supported use case yet, though we do have Support for Expo Web on our roadmap.

@kylegwalsh
Copy link
Author

@thiskevinwang That would make sense. We've actually been using clerk-expo since the beginning for mobile / web and never encountered any issues (until we tried to upgrade). The error definitely makes sense if the latest version of clerk-expo doesn't support web.

Happy to expand on our development flow as well. We actually created a single expo codebase that works on every platform (ios, android, and web). Most of the libraries that we're using support that setup, however we do have to occasionally create two separate files (index.web.ts vs. index.native.ts) to import separate packages based on platform. I can definitely modify the setup to import clerk-react on web if that should work.

@thiskevinwang
Copy link
Contributor

@kylegwalsh I've opened a PR to remove these headers - #3326

While that is in-review, you can also try clerk.__unstable__onBeforeRequest as a workaround to remove the headers before the request

@thiskevinwang
Copy link
Contributor

Hey @kylegwalsh this should be fixed in @clerk/[email protected]!

Give that a shot, and feel free to reopen this issue if the error persists!

npm install @clerk/clerk-expo@latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants