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

@clerk/remix integration returns a 401 + redirect when viewing a website after the short lived token has expired #2166

Closed
4 tasks done
AdiRishi opened this issue Nov 18, 2023 · 6 comments
Labels
linear Created by Linear-GitHub Sync

Comments

@AdiRishi
Copy link

AdiRishi commented Nov 18, 2023

Preliminary Checks

Reproduction / Replay Link

https://github.com/AdiRishi/clerk-remix-401-demo

Publishable key

pk_live_Y2xlcmsubmFhbWRlby5vcmck

Description

Description

The @clerk/remix integration seems to have behavior where when you visit a webpage with an expired short lived session token, the app will initially respond with a 401 code, then redirect the user back to the same page, finally loading with a 200.

Although this behavior won't be obvious to a user, for those of us trying to optimize the initial page load as much as possible, this creates a rather noticeable delay on the first page load (anywhere from 300ms to 1-2s).
Because the remix integration wraps the whole app, even content pages that need no auth will exhibit this behavior.

I personally feel like this is not really acceptable behavior. It should be possible to wrap your app in authentication and not have the first response always be a 401.
Looking forward to some feedback or advice on if this is a planned fix, or if there is no possible way to get around this.

Steps to reproduce

I've recorded examples from both my provided reproduction repository and one from the official clerk remix starter app found here.

Steps taken

  1. Log in to the app
  2. Close the browser tab with the webapp
  3. Wait for the short lived token to expire. My guess is you should wait around 30s to 1m?
  4. Go visit the webpage again

Expected Behavior

The web page should load with a 200 response code

Actual behavior

The web page initially loads with a 401 and then redirects back to itself and finally loads with a 200. My guess is the initial 401 response contains the now refreshed session token.

Environment

System:
  OS: macOS 14.1.1
  CPU: (8) arm64 Apple M1 Pro
  Memory: 575.31 MB / 32.00 GB
  Shell: 5.9 - /bin/zsh
Binaries:
  Node: 18.17.0 - ~/.nvm/versions/node/v18.17.0/bin/node
  Yarn: 1.22.19 - ~/.nvm/versions/node/v18.17.0/bin/yarn
  npm: 10.2.1 - ~/.nvm/versions/node/v18.17.0/bin/npm
  pnpm: 8.10.5 - ~/.nvm/versions/node/v18.17.0/bin/pnpm
Browsers:
  Chrome: 119.0.6045.159
  Safari: 17.1
npmPackages:
  @clerk/remix: ^3.1.5 => 3.1.5
  @remix-run/css-bundle: ^2.3.0 => 2.3.0
  @remix-run/dev: ^2.3.0 => 2.3.0
  @remix-run/eslint-config: ^2.3.0 => 2.3.0
  @remix-run/node: ^2.3.0 => 2.3.0
  @remix-run/react: ^2.3.0 => 2.3.0
  @remix-run/serve: ^2.3.0 => 2.3.0
  @types/react: ^18.2.37 => 18.2.37
  @types/react-dom: ^18.2.15 => 18.2.15
  eslint: ^8.54.0 => 8.54.0
  isbot: ^3.7.1 => 3.7.1
  prettier: ^3.1.0 => 3.1.0
  react: ^18.2.0 => 18.2.0
  react-dom: ^18.2.0 => 18.2.0
  typescript: ^5.2.2 => 5.2.2
@AdiRishi AdiRishi added the needs-triage A ticket that needs to be triaged by a team member label Nov 18, 2023
@jescalan jescalan added linear Created by Linear-GitHub Sync and removed linear Created by Linear-GitHub Sync labels Nov 20, 2023
@jescalan
Copy link
Contributor

We definitely understand your frustration here, and have heard it from others as well. We are currently actively looking into ways that we can change this behavior and still have everything work correctly. We are hoping to have a modified version out in the next few months that does not include this redirect behavior 🙌

However, as it stands, this is not a bug and the library still works correctly, so we're going to close this issue. But appreciate your surfacing this and again hoping to have it fixed soon!

@AdiRishi
Copy link
Author

Hey @jescalan , really glad to hear that this is a fix actively being worked on 😄 . Happy to do some early testing if needed, so don't hesitate to reach out.

Just a note, given that this issue is actually being worked on, it feels like it would be a good idea to leave this open with maybe a feature-request tag or something? But I totally understand the need to keep the issue list lean so all good either way.

@LekoArts LekoArts removed the needs-triage A ticket that needs to be triaged by a team member label Jan 8, 2024
@AdiRishi
Copy link
Author

AdiRishi commented Mar 8, 2024

Hey @jescalan I wanted to follow up again on this issue. You mentioned the team was researching a way to not have this behaviour, has there been any progress in this matter?

This is still a really annoying side effect of using Clerk, one that causes much grief when we work hard on website performance. Would really love to know what is being done to solve this.

@jescalan
Copy link
Contributor

jescalan commented Mar 8, 2024

Hi there! So sorry for the slow response here @AdiRishi. There has been indeed, we have changed this mechanism in the latest beta of the remix SDK.

Give it a shot and see if this does the trick for you.

Also note that the previous behavior (interstitial) was replaced by a new behavior (handshake) that runs faster and does not render an actual page in its redirect cycle, but will still redirect a short loop if it detects an expired session token. There isn't a way for us to remove the need to re-validate if there's an expired session token the way that clerk's auth is architected. This should pay off overall in your page render speed however, since your backend only needs to verify the token's signature, which is very fast (usually ~1ms or less), rather than making a database call to verify auth state, which is much slower. Happy to go into more detail on this if you're curious.

@AdiRishi
Copy link
Author

@jescalan Sorry for the super delayed response, I just got to updating the production sites that use Clerk to the latest version with Clerk Core 2. The update's working really well, flash is gone, super smooth experience 😄
Getting server responses under 100ms for my page loads, which is fantastic 🎉

@jescalan
Copy link
Contributor

jescalan commented May 6, 2024

Amazing, this is so great to hear! Thank you so much for the feedback here and for using Clerk 💖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear Created by Linear-GitHub Sync
Projects
None yet
Development

No branches or pull requests

3 participants