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

Inconsistent Exception/Error Handling (500 Internal Server Error / custom error / _error.js) #18209

Open
Zauberfisch opened this issue Oct 24, 2020 · 0 comments
Assignees
Labels
Linking and Navigating Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@Zauberfisch
Copy link

Bug report

Depending on where an error occurs, I've noticed at least 4 different behaviours that happen in response to an error:

I've created a minimal app with the following files: pages/_app.js, pages/_error.js, pages/[[...slug]].js and tested the following (in production build):

  1. throw in _app.js getInitialProps
    • response on page load: Internal Server Error (string, no html document)
    • response on (client side) navigation: redirect/page-reload and then Internal Server Error (string, html document)
  2. throw in _app.js render
    • response on page load: Internal Server Error (string, no html document)
    • response on (client side) navigation: exception happens client side, resulting in endless loop of react trying to render the component (browser becomes unresponsive)
  3. throw in pages/[[...slug]].js getServerSideProps
    • response on page load: content of _error.js
    • response on (client side) navigation: redirect/page-reload and then content of _error.js
  4. throw in pages/[[...slug]].js render (or any sub component)
    • response on page load: content of _error.js
    • response on (client side) navigation: content of _error.js
  5. api routes will crash next, see Next.js production build doesn't prevent process exiting due to unhandled exceptions #17574

To Reproduce

// pages/_app.js
import Link from 'next/link'

function App({Component, pageProps, router}) {
	if (router.query.testerror === 'app') {
		throw '[testerror] app';
	}
	return (<>
		<div style={{padding: 20}}>
			{['/', '/?testerror=app', '/?testerror=app-getInitialProps', '/?testerror=page', '/?testerror=page-getServerSideProps'].map((url, i) => (
				<>
					<Link key={url} href={url}><a>{url}</a></Link>
					{' | '}
				</>
			))}
		</div>
		{pageProps?.myStatusCode ? `pageProps.statusCode in _app.js = ${pageProps?.myStatusCode}` : <Component {...pageProps} />}
	</>)
}

App.getInitialProps = async ({router}) => {
	if (router.query.testerror === 'app-getInitialProps') {
		throw '[testerror] app-getInitialProps';
	}
	return {}
}

export default App
// pages/_error.js
function Error(props) {
	return <div>
		<h1>_error.js</h1>
		<pre>{JSON.stringify(props, null, 4)}</pre>
	</div>
}

Error.getInitialProps = ({res, err}) => {
	const statusCode = res ? res.statusCode : err ? err.statusCode : 404
	return {statusCode}
}

export default Error
// pages/[[...slug]].js
import {useRouter} from "next/router";

export default function RequestHandler(props) {
	const {query} = useRouter()
	if (query.testerror === 'page') {
		throw '[testerror] page';
	}
	return (
		<div>
			Page
			<pre>{JSON.stringify(props, null, 4)}</pre>
		</div>
	)
}

export async function getServerSideProps({params, req, res, query, preview, previewData, resolvedUrl}) {
	if (query.testerror === 'page-getServerSideProps') {
		throw '[testerror] page-getServerSideProps';
	}
	if (!params.slug || ['page-a', 'page-b'].includes(params.slug[0])) {
		return {
			props: {
				content: `This is the ${params.slug ? params.slug[0] : 'index'} page`
			}
		}
	}
	if (!req.url.match(/^\/_next\/data\/.*slug=.*$/)) {
		// only set status code 404 if it's a page load, not for XHR, see #18185
		res.statusCode = 404
	}
	return {
		props: {
			myStatusCode: 404,
			myMessage: `couldn't find page`
		}
	}
}

Example code includes Links to different URLs with exceptions

Expected behavior

  • exceptions in _app.js should also be caught and handled the same as in pages
  • exceptions in app render should not cause a rendering loop which makes the browser become unresponsive
  • failing the above two points, we should update the documentation to mention that there are cases where custom error handling will not work

And if the mentality is really like described in the first comment on #17574 "let it crash", then why do we have _error.js. I'm not arguing for either side, but as a project next should commit to one way and do it as consistently as possible (and clearly document cases where it's not consistent).

System information

  • Version of Next.js: 9.5.5
  • Version of Node.js: v14.12.0
@timneutkens timneutkens added this to the Iteration 19 milestone Apr 12, 2021
@timneutkens timneutkens modified the milestones: Iteration 19, Iteration 20 May 3, 2021
@styfle styfle modified the milestones: 11.1.x, 12.0.4 Nov 5, 2021
@timneutkens timneutkens added the Linking and Navigating Related to Next.js linking (e.g., <Link>) and navigation. label Nov 16, 2021
@timneutkens timneutkens removed this from the 12.0.5 milestone Nov 17, 2021
@samcx samcx removed the kind: bug label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linking and Navigating Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

No branches or pull requests

5 participants