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.

Sign up for GitHub

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

Jump to bottom

fix: cannot set headers after they are sent to the client #875

Merged
merged 5 commits into from
Sep 5, 2024
Merged

fix: cannot set headers after they are sent to the client #875

merged 5 commits into from
Sep 5, 2024

Conversation

zoey-kaiser
Copy link
Member

@zoey-kaiser zoey-kaiser commented Aug 22, 2024 •
edited
Loading

🔗 Linked issue

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This PR fixes a long-lasting bug. On first redirect from a protected pages to the login page using the authjs provider, you would often get the following message in the console:

[nuxt] [request error] [unhandled] [500] Cannot set headers after they are sent to the client

This issue originates from the problem of how do we set a redirect inside the SSR context if the page is protected. Until now we used a outdated workaround from Nuxt. I did some digging and managed to find a better solution here.

The difference

In the old version we did the following:

  • Check if we are calling the function from the server using import.meta.server
  • Then we check if both the nuxtApp.ssrContext and nuxtApp.ssrContext.event were set
  • Then we use a h3 redirect on the server-side

The issue then occurred when the client-side redirect occurs different to the server-side redirect. At that point the client would already be redirected, but the h3 event would still try and set the headers. Resulting in the error.

In the new version I remove the dependency on h3 completely and do the following:

  • Check if we are calling the function from the server using import.meta.server
  • Check if ONLY the nuxtApp.ssrContext is set (will be set on SSR)
  • Replace the pre-rendered page with a HTML meta redirect
    • This ensures we return a valid means of redirecting as the ssr-rendered page
    • We do not relay on 2 different redirects (client and ssr)

I set the following as the ssr-rendered response:

const encodedLoc = href.replace(/"/g, '%22')
const encodedHeader = new URL(href).toString()

nuxtApp.ssrContext!._renderResponse = {
  statusCode: sanitizeStatusCode(302, 302),
  body: `<!DOCTYPE html><html><head><meta http-equiv="refresh" content="0; url=${encodedLoc}"></head></html>`,
  headers: { location: encodedHeader },
}

When using curl to get the initially SSR-render content, we can also see that the responses are the same. So we are functionally doing the same thing as before, just cutting out h3 and setting the content ourselves to avoid issues.

Main

Screenshot 2024-08-22 at 21 21 54

This PR

Screenshot 2024-08-22 at 21 18 55

This solution works, maintains protection and ensures that the error does not occur anymore!

Verification

To test that this resolves the issue:

  • I build the app from main and started the production build and received the error after directly accessing a protected URL (in my case /protected/globally)
  • I then built the app from this PR and also started the production build and did not receive the error!

Main:

Screen.Recording.2024-08-22.at.21.14.23.mov

This PR:

Screen.Recording.2024-08-22.at.21.13.21.mov

📝 Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

@zoey-kaiser zoey-kaiser added bug A bug that needs to be resolved provider-authjs An issue with the authjs provider labels Aug 23, 2024
@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Aug 25, 2024 •
edited
Loading

Open in Stackblitz

pnpm add https://pkg.pr.new/@sidebase/nuxt-auth@875

commit: 39545a7

@zoey-kaiser zoey-kaiser merged commit d34848b into main Sep 5, 2024
7 checks passed
@zoey-kaiser zoey-kaiser deleted the fix/cannot-set-headers branch September 5, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phoenix-ru phoenix-ru phoenix-ru approved these changes

Assignees
No one assigned
Labels
bug A bug that needs to be resolved provider-authjs An issue with the authjs provider
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zoey-kaiser @phoenix-ru