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

Rewrites in next.config.js doesnt work #263

Closed
yarinsa opened this issue Oct 10, 2023 · 25 comments · Fixed by #411
Closed

Rewrites in next.config.js doesnt work #263

yarinsa opened this issue Oct 10, 2023 · 25 comments · Fixed by #411

Comments

@yarinsa
Copy link

yarinsa commented Oct 10, 2023

After updating Next.js to 13.5.3 to handle a bad error in production (500, attemp to render client in the server). The following stopped working:
redirects: () => {
const SIGNIN_ROUTE = /login;
const DEFAULT_ROUTE = /dashboard;
return [
{
source: /,
destination: DEFAULT_ROUTE,
permanent: true,
has: [{type: cookie, key: next-auth.session-token}],
},
{
source: /,
destination: DEFAULT_ROUTE,
permanent: true,
has: [{type: cookie, key: __Secure-next-auth.session-token}],
},
{
source: /,
destination: SIGNIN_ROUTE,
permanent: true,
missing: [{type: cookie, key: __Secure-next-auth.session-token}],
},
{
source: /,
destination: SIGNIN_ROUTE,
permanent: true,
missing: [{type: cookie, key: next-auth.session-token}],
},
];

I am in a damn loop hole, for few weeks already.
My original issue was Next trying to generated protected pages during ISG, and then caching a 500 result.
Then I updated next.js and sst to the latest stable versions (13.5.4, sst 2.26.10).
My folder setup is as following:
src
├── app
│ ├── @authenticated
│ ├── @login
│ ├── _components
│ ├── _hooks
│ ├── _lib
│ ├── api
│ ├── layout.tsx
│ ├── loading.tsx
│ ├── page.tsx
│ └── providers.tsx
└── middleware.ts

Then my root layout conditionally renders login/auth.
Everything works as expected except the root dir "/" which is an empty route.
NextAuth should redirect to '/login' and next itelsf to 'dashboard'.
It works locally without setting next.config.ts.
I tried adding the same login in the middleware but it doesnt seem like the middleware is even running (not seeing any logs form there).
It gets a successful response from '/api/auth/session' but the redirect doesnt happen :/ Any ideas?

@khuezy
Copy link
Collaborator

khuezy commented Oct 10, 2023

Should be fixed in #261, will release 2.2.2 soon. If not please reopen

@khuezy khuezy closed this as completed Oct 10, 2023
@yarinsa
Copy link
Author

yarinsa commented Oct 10, 2023

Thank you 🙏🏼🙏🏼🙏🏼

@yarinsa
Copy link
Author

yarinsa commented Oct 19, 2023

@khuezy Should we reopen as the issue still exists in sst 2.30.2 and next 13.5.6

@khuezy
Copy link
Collaborator

khuezy commented Oct 19, 2023

See: #261 (comment)

According to the E2E test, main passes 13.5.6
https://github.com/sst/open-next/actions/runs/6568405904/job/17842773477
41/41 passed (1 was flaky)

@topaxi
Copy link

topaxi commented Nov 14, 2023

I'm still experiencing that (some) rewrites do not work with next.config.js and SST, it seems like they don't work at all and I get 404 errors.

Rewrites are all simple "afterFiles" rewrites with source, destination and locale: false (no cookies or other special options).

Downgrading back to Next.js 13.4 (without touching SST etc.) works.

I can try to build a small reproduction later this month.

@khuezy
Copy link
Collaborator

khuezy commented Nov 14, 2023

There is an error w/ external rewrites, we can reopen this ticket.

@khuezy khuezy reopened this Nov 14, 2023
@yarinsa
Copy link
Author

yarinsa commented Nov 15, 2023

@topaxi
How did you debug this ? Or saw the error?
I am stuck with next 13,4 that takes lots of memory

@conico974
Copy link
Collaborator

@yarinsa Did you try with latest open-next ? I've managed to reproduce your issue and pushed a bunch of fix in #299. There is still an issue when the target send chunked data though

@khuezy
Copy link
Collaborator

khuezy commented Nov 17, 2023

The types might be incorrect. .getHeaders doesn't exist, it's in req.headers.
When a site has encoding, the response renders a bunch of symbols. I sort of have half a solution but it doesn't work in all scenarios... It's a bit tricky, especially when the site uses prefetch or dynamic relative resources

@conico974
Copy link
Collaborator

The types might be incorrect. .getHeaders doesn't exist, it's in req.headers. When a site has encoding, the response renders a bunch of symbols. I sort of have half a solution but it doesn't work in all scenarios... It's a bit tricky, especially when the site uses prefetch or dynamic relative resources

One things that is really weird is that the same code work locally, but not on lambda. During my test i've tried running a fetch in lambda and a fetch locally, in lambda it returns a bunch of symbols and locally it works fine. Not sure what's going on here

@yarinsa
Copy link
Author

yarinsa commented Nov 19, 2023

@yarinsa Did you try with latest open-next ? I've managed to reproduce your issue and pushed a bunch of fix in #299. There is still an issue when the target send chunked data though

I did

@caaatisgood
Copy link
Contributor

caaatisgood commented Nov 29, 2023

I had the same issue on open-next@2.3.2 and next@14.0.3. But the rewrites seem to work fine after bumping to open-next@2.3.3. Perhaps this patch fixed the problem for me.


Update:

The simple rewrites such as below works

const nextConfig = {
  rewrites: async () => [
    { source: "/rewrite", destination: "https://github.com" },
  ],
}
module.exports = nextConfig

But not for the more complex one which has query string involved. Take Sentry as an example (source):

const injectedRewrite = {
  // Matched rewrite routes will look like the following: `[tunnelPath]?o=[orgid]&p=[projectid]`
  // Nextjs will automatically convert `source` into a regex for us
  source: `${tunnelPath}(/?)`,
  has: [
    {
      type: 'query',
      key: 'o', // short for orgId - we keep it short so matching is harder for ad-blockers
      value: '(?<orgid>\\d*)',
    },
    {
      type: 'query',
      key: 'p', // short for projectId - we keep it short so matching is harder for ad-blockers
      value: '(?<projectid>\\d*)',
    },
  ],
  destination: 'https://o:orgid.ingest.sentry.io/api/:projectid/envelope/?hsts=0',
};

@yarinsa
Copy link
Author

yarinsa commented Dec 7, 2023

@caaatisgood ~~Thanks, I will try this and update back here ~~
It does work :) now I got some other sentry error preventing me from updating next :( but I think this issue can be closed

@topaxi
Copy link

topaxi commented Jan 5, 2024

Retried with next@14.0.4, sst@2.29.2 and open-next@2.3.3, none of our rewrites in next.config.js seem to work.

Haven't had the time yet to create a minimal reproduction, this currently blocks us in several ways, so I hope I can spend some time to debug further and provide a reproduction.

@ianschmitz
Copy link

Take Sentry as an example (source):

const injectedRewrite = {
  // Matched rewrite routes will look like the following: `[tunnelPath]?o=[orgid]&p=[projectid]`
  // Nextjs will automatically convert `source` into a regex for us
  source: `${tunnelPath}(/?)`,
  has: [
    {
      type: 'query',
      key: 'o', // short for orgId - we keep it short so matching is harder for ad-blockers
      value: '(?<orgid>\\d*)',
    },
    {
      type: 'query',
      key: 'p', // short for projectId - we keep it short so matching is harder for ad-blockers
      value: '(?<projectid>\\d*)',
    },
  ],
  destination: 'https://o:orgid.ingest.sentry.io/api/:projectid/envelope/?hsts=0',
};

I can confirm i'm observing the same issue with trying to configure Sentry with tunnelRoute config which adds the rewrite above. I see the following error:

Invoke Error	{
  "errorType": "TypeError",
  "errorMessage": "Invalid URL",
  "code": "ERR_INVALID_URL",
  "input": "https://o:orgid.ingest.sentry.io/api/:projectid/envelope/%3Fhsts=0",
  "stack": [
    "TypeError: Invalid URL",
    "    at new URL (node:internal/url:775:36)",
    "    at uW (file:///var/task/packages/web/index.mjs:55:1679)",
    "    at eu (file:///var/task/packages/web/index.mjs:55:3553)",
    "    at QQ (file:///var/task/packages/web/index.mjs:55:10516)",
    "    at async ZQ (file:///var/task/packages/web/index.mjs:55:12038)"
  ]
}

@debu99
Copy link

debu99 commented Feb 19, 2024

same issue on next@14.1.0, open-next@2.3.5

@raducostea
Copy link

Not working in both middleware or next.config.js in next@14.1.0

@khuezy
Copy link
Collaborator

khuezy commented Mar 1, 2024

@ianschmitz looks like open-next isn't properly passing the regex group to the destination URL

eg: "input": "https://o:orgid.ingest.sentry.io/api/:projectid/envelope/%3Fhsts=0"

Can you help debug why around this function: https://github.com/sst/open-next/blob/main/packages/open-next/src/adapters/routing/matcher.ts#L122-L170

@conico974
Copy link
Collaborator

There is a workaround for the sentry issue by using the middleware

if (request.nextUrl.pathname.startsWith('/monitoring')) {
    const orgId = request.nextUrl.searchParams.get('o');
    const projectId = request.nextUrl.searchParams.get('p');
    return NextResponse.rewrite(new URL(`https://o${orgId}.ingest.sentry.io/api/${projectId}/envelope/?hsts=0`));
  }

We need to be careful with this sentry rewrite, if we don't do it properly it can create a security risks since people could use this to rewrite to external url.

@raducostea @debu99 First things to check is if you're building on windows, open-next doesn't work well there and it causes a bunch of weird issues. You can use WSL there it should work.
If this doesn't help you need to at least provide a reproduction so that we can debug this. There is e2e tests with rewrites, we could add those missing cases

@danjiro
Copy link

danjiro commented Mar 10, 2024

following as rewrites is confirmed still not working with next 14.1.0

@conico974
Copy link
Collaborator

@danjiro Could you provide a reproduction ?
A reproduction of the issue or a PR to fix it are essential for finding a solution. Without them, it is very unlikely that the problem will be resolved.

@danjiro
Copy link

danjiro commented Mar 10, 2024

A simple rewrite works locally but not when deployed to AWS. Using Next14.1.0, sst 2.40.6.

async rewrites() {
    return [
      {
        source: '/blog',
        destination: 'https://blog.example.com',
      },
    ];
  },

gets ERR_CONTENT_DECODING_FAILED as the status in the network tab. nothing is rendered

@conico974
Copy link
Collaborator

@danjiro This issue should be fixed in V3 rc, could you test it and see if you still have the issue.
The easiest way to test is probably to use ion which already uses V3 rc https://ion.sst.dev/

This issue doesn't always occur as well, i think the issue exists when the external target rewrites uses br or gzip encoding by default

conico974 added a commit to conico974/open-next that referenced this issue May 9, 2024
conico974 added a commit to conico974/open-next that referenced this issue May 12, 2024
conico974 added a commit that referenced this issue May 12, 2024
* should fix most of #263

* added e2e test

* directly return

* Create fluffy-lamps-confess.md
@ghardin1314
Copy link

ghardin1314 commented May 23, 2024

@conico974 I believe I am still running into this issue when trying to implement a proxy for posthog analytics. It uses external rewrites and gzip encoding

Using Next v14.2.3 and OpenNext v3.0.0-rc.16

my next.config looks like

{
async rewrites() {
    return [
      {
        source: "/ingest/static/:path*",
        destination: "https://us-assets.i.posthog.com/static/:path*",
      },
      {
        source: "/ingest/:path*",
        destination: "https://us.i.posthog.com/:path*",
      },
    ];
  },
  // This is required to support PostHog trailing slash API requests
  skipTrailingSlashRedirect: true,
}

and I am only getting a 400 bad request error back. Can't figure out how to troubleshoot further.

Following these docs as a guide

I think it may be the gzip specifically because a few requests with base64 compression still work find

@conico974
Copy link
Collaborator

@ghardin1314 Could you create a new issue ? There is like 5 different issues here, and all of them are solved or does not have a reproduction.
To help with debugging you could deploy using OPEN_NEXT_DEBUG=true sst deploy, you'll get a ton of logs that might help pinpoint the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants