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

The redirect function allows to avoid a parameter for the Location #15594

Closed
AndyBitz opened this issue Jul 28, 2020 · 1 comment · Fixed by #15844
Closed

The redirect function allows to avoid a parameter for the Location #15594

AndyBitz opened this issue Jul 28, 2020 · 1 comment · Fixed by #15844
Labels
good first issue Easy to fix issues, good for newcomers
Milestone

Comments

@AndyBitz
Copy link
Contributor

Bug report

Describe the bug

The redirect function added in #14705 allows to set a status code or URL as second argument, and an optional third argument for the URL if the second argument was used for the status code.

However, the function signature allows setting a status code without providing a URL. In those cases the Location header would be set to undefined, which throws an error.

To Reproduce

  • Create a Next.js project with TypeScript
  • Create pages/api/index.ts with this content:
import { NextApiRequest, NextApiResponse } from 'next';
export default (req: NextApiRequest, res: NextApiResponse) => res.redirect(307);
  • Run next dev and execute curl localhost:300/api
  • 500 Status code is returned and error is logged
TypeError [ERR_HTTP_INVALID_HEADER_VALUE]: Invalid value "undefined" for header "Location"
    at ServerResponse.setHeader (_http_outgoing.js:521:3)
    at setHeadersFromObject (/private/tmp/nextjs/node_modules/next/dist/compiled/compression/index.js:1:145242)
    at ServerResponse.setWriteHeadHeaders (/private/tmp/nextjs/node_modules/next/dist/compiled/compression/index.js:1:145476)
    at ServerResponse.writeHead (/private/tmp/nextjs/node_modules/next/dist/compiled/compression/index.js:1:144683)
    at redirect (/private/tmp/nextjs/node_modules/next/dist/next-server/server/api-utils.js:32:114)
    at ServerResponse.apiRes.redirect (/private/tmp/nextjs/node_modules/next/dist/next-server/server/api-utils.js:6:340)
    at __webpack_exports__.default (webpack-internal:///./pages/api/index.js:2:82)
    at apiResolver (/private/tmp/nextjs/node_modules/next/dist/next-server/server/api-utils.js:8:7)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5) {
  code: 'ERR_HTTP_INVALID_HEADER_VALUE'
}

Expected behavior

TypeScript should have caught the error upfront.

Personally I think it'd make more sense to have a signature like this:

function redirect(url: string, status: number = 307);

so that no matter what, the Location will always be set and won't depend on the type. But this might be impossible to change now. I'm not sure if it'd make sense to throw a custom error if the URL parameter was forgotten.

Maybe it'd also be enough to rely on the number of arguments instead of their type.

System information

  • OS: macOS
  • Version of Next.js: 9.5.0
  • Version of Node.js: v12.18.3
@Timer Timer added this to the iteration 6 milestone Jul 28, 2020
@timneutkens timneutkens added the good first issue Easy to fix issues, good for newcomers label Jul 28, 2020
@Timer Timer modified the milestones: iteration 6, backlog Jul 28, 2020
@kodiakhq kodiakhq bot closed this as completed in #15844 Aug 6, 2020
kodiakhq bot pushed a commit that referenced this issue Aug 6, 2020
## Which solves this PR

 Displaying a friendly error, when the user is only passing `statusOrUrl`(type number) and the second argument `url` is ignored.

**Example**

`res.redirect(307);` // Show friendly error

Closes: #15594
x-ref: #15603
@Timer Timer modified the milestones: backlog, iteration 6 Aug 6, 2020
styfle added a commit to vercel/vercel that referenced this issue Aug 18, 2020
This pull request adds a method redirect, the behavior will be the same as that of Next.js with `pages/api`. This PR takes into account the change that is being made in Next.js with redirect due to the [error that was reported](vercel/next.js#15594) a few hours ago.

Co-authored-by: Steven <steven@ceriously.com>
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Easy to fix issues, good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants