Hi folks, really loving Next so far, just ran into one issue:
When I redirect on the server from getInitialProps, like this:
import React from 'react'
export default class extends React.Component {
static getInitialProps ({ res }) {
// assume we're on the server for this example, obviously this won't work on the client
res.redirect('/')
return {}
}
render () {
return <h1>Hello</h1>
}
}
I get a server-side error:
Error: Can't set headers after they are sent.
at ServerResponse.setHeader (_http_outgoing.js:371:11)
at sendHTML (/Users/booker/Code/next-minimal-oauth-authentication-demo/node_modules/next/dist/server/render.js:422:7)
at Server._callee17$ (/Users/booker/Code/next-minimal-oauth-authentication-demo/node_modules/next/dist/server/index.js:767:73)
at tryCatch (/Users/booker/Code/next-minimal-oauth-authentication-demo/node_modules/regenerator-runtime/runtime.js:65:40)
at Generator.invoke [as _invoke] (/Users/booker/Code/next-minimal-oauth-authentication-demo/node_modules/regenerator-runtime/runtime.js:303:22)
at Generator.prototype.(anonymous function) [as next] (/Users/booker/Code/next-minimal-oauth-authentication-demo/node_modules/regenerator-runtime/runtime.js:117:21)
at step (/Users/booker/Code/next-minimal-oauth-authentication-demo/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:30)
at /Users/booker/Code/next-minimal-oauth-authentication-demo/node_modules/babel-runtime/helpers/asyncToGenerator.js:28:13
> Building page: /
Also note that this code works (the user is redirected to /), but I see this error in the console. Weird!
Also note that I'm using express and next (the npm lib, not the cli)
Any thoughts?
I'm not sure if you can do a redirect like this inside of Next.js on the server side, but curious to know if that is possible.
There is next/router on the client, but I'm not sure that it runs server side (it doesn't AFAIK):
import Router from 'next/router'
Router.push('/redirect-route')
I wonder if it's worth adding a route handler in express to take of server side redirection?
server.get('/example-route', (req, res) => {
return res.redirect(301, 'https://example.com/redirect-route')
})
Oh, it is definitely possible! Andnext/router is definitely for the client only. If you attempt to run it from the server, you'll get an error.
I too _suspect_ that the solution here is to define my routes on the server however, for context, I am working on an authentication module called ensureAuthenticated that looks like this:
const ensureAuthenticated = Component => {
return class extends React.Component {
static async getInitialProps ({ req, res }) {
let session = {}
if (req) {
if (req.user) {
session = {
user: req.user
}
} else {
res.redirect('/admin/login') // throws harmless but irksome "Can't set headers after they are sent" error, curious why and how to eliminate it!
}
} else {
const response = await fetch('/auth/session', {
credentials: 'same-origin'
})
const json = await response.json()
if (json.user) {
session = {
user: json.user
}
} else {
Router.push('/admin/login')
}
}
let isAuthenticated
if (session.user && session.user.id) {
isAuthenticated = true
}
return { isAuthenticated, session }
}
render () {
if (this.props.isAuthenticated) {
return <Component {...this.props} />
}
return null
}
}
}
As you can see, there is a certain symmetry to my client and server authentication logic that I'm attached to (maybe I shouldn't be)
Like I say, it works, I just get an error in the terminal which bothers me.
If you'd like a reference for working universal auth with next and express there is one at nextjs-starter.now.sh FWIW.
I haven't been able to figure out how to simplify it without dropping features though as there were just too many cases to cater for so the session.js component end up being quite extensive.
I also ran into req and res being slightly different in getInitialProps() to the values exposed in express.
hehe, this _is_ based on starter.now.sh. I'm endeavouring to build a simpler example, focusing on one OAuth provider only. Great work on that!
Just published the latest version if you want to check it out: https://github.com/bookercodes/next-minimal-oauth-authentication-demo
Remember to yarn install and to create .env if you want to run it locally:
GOOGLE_CLIENT_ID=
GOOGLE_CLIENT_SECRET=
SESSION_SECRET=123123123123123123123123123123123123123123123123
PORT=8080
It works really well aside from a few harmless but irksome errors in the terminal. I counted three (harmless) errors so far, will work through them and see if a better "structure" emerges
@bookercodes @iaincollins https://github.com/timneutkens/next-universal-redirect/blob/master/index.js#L6-L30 I made this a while back ๐ And haven't had the errors you're seeing.
@timneutkens I got the same error as described in the OP and saw it disappeared when removing my express-session middleware. Haven't had yet time to debug more.
Thanks @timneutkens!
@sedubois aah yes that rings a bell, thanks for the tip.
@timneutkens Will check it out, thanks
@sedubois Interesting. Can't remove express-session without breaking my app though!
Would be great to test if tim's package combined with express-session shows the issue. If yes, then this ticket could be reopened?
@sedubois either way it's related to express And not next.js cause without that package it works right. So in theory you could reproduce it with a simple request handler that does the same thing as getInitialProps.
@timneutkens I'm not really following you. Do you mean that server.get('/test', (req, res) => res.redirect('/admin/login') should throw the same error? It doesn't
@sedubois Yeah, I just tried
res.writeHead(301, {
Location: '/admin/login'
})
res.end()
... Same error :(
Really appreciate you all trying to help by the way!
@bookercodes Hmmm, okay, I'll re-open this then, cause we're still discussing it ๐
@bookercodes one thing I'm thinking right now is, have you tried production mode? We can continue the conversation on slack if you'd like ๐
@timneutkens Yeah, same problem :(
I am starting to notice that this happens sporadically now, eek!
Just tested, and this happens with Hapi too.
Also tested next@beta as well as the latest stable release (just in case)
Also having this issue... was working 5 minutes ago.
@bookercodes @timneutkens are you setting res.finished = true after you've sent the response? e.g.
if (res) {
res.redirect(`/article/${doc.id}/${doc.slug}`)
res.finished = true
}
return {}
No, I'm not. I can't remember off the top of my head, but doesn't res.redirect do that automatically?
@bookercodes res.redirect will set the 301 status code, write the location header and end the response. However res.finished is a next.js construct only, that tells next.js you have handled the entire request/response lifecycle in getInitialProps, so that it knows not to continue writing to the response.
See https://github.com/zeit/next.js/blob/87e01f681bc2d82e59ee30c026180d6984157a34/server/render.js#L67
@mattfysh does that mean doing that works? ๐ In that case thanks ๐ค
@timneutkens yea this is working for me now :)
Oh wow, good to know.
Will test this this afternoon :)
Amazing. TIL ๐ฒ
Quick comment on this issue:
When using res.finished = true the connection never seemed to finish and ran till timeout.
When i replaced res.finished = true with res.end() it seemed to work.
I got that idea from this example -
https://github.com/zeit/next.js/blob/dff20eae0137ee121c703733fbb87084a8cd86d3/test/integration/basic/pages/nav/redirect.js
I'm unsure if this is the correct way to do this, but it worked in a way that the code on the wiki page did not.
You need to do both.
On Mon, 17 Jul 2017 at 9:23 pm, dajomu notifications@github.com wrote:
Quick comment on this issue:
When using res.finished = true the connection never seemed to finish and
ran till timeout.
When i replaced res.finished = true with res.end() it seemed to work.
I got that idea from this example -I'm unsure if this is the correct way to do this, but it worked in a way
that the code on the wiki page did not.โ
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/zeit/next.js/issues/2152#issuecomment-315729132, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAcs8ovGCkAwNcOYm7z3wyuaNHIs5Avwks5sO0QygaJpZM4Nuxwb
.
Ah, ok, thanks
After adding res.finished = true I get the timeout issue again, so I've removed it for now.
@dajomu hmmm... this doesn't sound right, if you're getting a timeout its because you're not handling the request/response lifecycle correctly.
Setting res.finished = true tells next.js you've already processed the entire request lifecycle, and that it does not need to do anything further with the req or res objects. Can you upload an example repo?
This should work:
res.finished = true
res.end()
@dajomu I noticed the wiki only had writeHead and res.finished = true ... which will result in the timeout because the response was never ended. I've updated the wiki example - https://github.com/zeit/next.js/wiki/Redirecting-in-%60getInitialProps%60
Thanks @mattfysh
Just out of interest, is this something that can/will be abstracted by Next?
Having to write,
res.finished = true
res.end()
Is neither intuitive nor "clean", you know?
Not a complaint as I am aware of the solution (and I don't feel like I'm in a position to propose a change), just advocating for future Next users who might trip over this.
I wonder if this need to write res.finished = true, res.end() stems from something internal that could be tweaked?
Perhaps next could pass in a redirect function and make the request/response management opaque to getInitialProps?
Just for the record: in my case, the same error appeared when using compression middleware with a custom express server.
Most helpful comment
@bookercodes @timneutkens are you setting
res.finished = trueafter you've sent the response? e.g.