Catch-all at /pages root breaks static file serving

[patricktyndall]

Describe the bug

Next supports having a catch-all route at pages/[...all] (this works with next start). However, when I add this file (with getStaticPaths fallback: true) and deploy using serverless-next.js, requests for static assets break.

Actual behavior

Assets don't load

• in some cases, they 503
• in other cases, it seems requests for static data are handled by pages/[...all] (next's fallback rendering). This results in a 200 with type text/html. I can paste these asset URLs in my browser and get the fallback page.

This applies to all kinds of assets (next's /_next files as well as things in public/).

Expected behavior

Static files are served correctly.

Steps to reproduce

Add a catch-all route at pages/[...all] with getStaticPaths. (I haven't tested this with SSR)

Versions

• OS/Environment: Mac
• @sls-next/serverless-component version: 1.17.0
• next: 9.4.4

Other info

When I add fallback: false the requests become 503s, but otherwise the issue is the same.

Does serverless-next.js support this? I couldn't find mention in the documentation, sorry if I missed something. It makes sense that this would be tricky, but wondering if there's a known workaround.

If this is unsupported, is there a roadmap or further discussion on this? This is very high-value to my team, as it would unlock the ability to define any URL path using CMS.

---

ETA:

• If it's helpful, my cloudwatch logs are full of this error
  
• Also, I tried this with a dynamic, non-catch-all route (pages/[all]) and it still breaks for public/test.png, but no longer breaks for public/assets/test.png or for /_next assets. This seems to confirm the rule that assets URLs can't match /pages patterns.

My solution for now is to both a) use [all] for content pages (ie limit created pages to one path segment), and b) require everything in public to be in public/assets so they don't match [all]

[dphang]

Thanks, I will take a look at this this week.

I think this component should support it according to features list but I believe catch-all and getStaticPaths were each implemented separately, perhaps there's a bug when both are used.

[patricktyndall]

Hey @dphang I found something worse: Adding a dynamic SSG route at pages/[all] breaks all server-side data fetching for other pages/.

• ex: I have a pages/about (with getServerSideProps) and a pages/[all] (with getStaticPaths/Props). When I visit pages/about, its props are empty and getServerSideProps has not run (pageProps is {}). If I rm pages/[all] and redeploy, /about.getServerSideProps runs fine.
• The above applies when /about is using getInitialProps too

So, to summarize my understanding of the issue:

• When the deployed URLs of things in public/ or pages/ match the deployed URL pattern for a getStaticPaths, those assets and pages are served incorrectly (getStaticPaths is interfering with these requests, when it should not be)

This is fatal for my current project as I had promised complete control over URLs, based on Next's support for this use case (seems like an important break in compatibility, no?). Any idea how soon this can be addressed?

[dphang]

Sure, sorry I got busy this weekend but I will take a look some time this week. I don't use catch-all (and didn't write the feature) and unfortunately we don't have e2e test coverage for catch-all yet, but I'd like to add some test coverage as well so this doesn't break in the future.

[patricktyndall]

No worries, thanks for the work. It seems the issue is getStaticPaths generally, not just catch-all.

---

Another question: As a workaround, I attempted to achieve my "clean URLs" goal using the new rewrites functionality (using 1.18.0-alpha.4). I moved the dynamic route to /pages/content-pages/[all], and added the below to rewrites:

return [{
	source: '/:path*',
	destination: '/:path*',
},{
	source: '/:path*',
	destination: '/content-pages/:path*',
}];

This works when I build; however, on deployed site, when I visit /my-dynamic-route, I get a 404 with "An unexpected error has occurred." (I can visit /content-pages/my-dynamic-route though).

Can you confirm this approach should work? Are there any "gotchas" to know about when getting this working? (ex: I have not added custom domain config to serveless.yml). I'm going off of this issue.

[dphang]

Could you please check CloudWatch and post the logs? It may be a bug in the rewrite code - it's in alpha now and I have tests for it, but it seems it could still have issues.

[dphang]

Published a fix in: https://www.npmjs.com/package/@sls-next/serverless-component/v/1.18.0-alpha.6, please try out and see if it works.

[patricktyndall]

Hey @dphang thanks for the update. Unfortunately, I'm getting 404 when I access /my-dynamic-route using the rewrite example setup above. Attached a screenshot of the logs (they all look like this, even from previous versions).

Some other info, in case it's helpful:

• Client-side routing to these pages works though (json files load)
• Static routes still work fine ie /login
• I don't think it should matter, but I have defined _app.getInitialProps. I can see it being invoked on the _error page that results from the above 404, but not on the 404 request itself

To clarify: I have not tested under the original issue case of catch all at /pages/[...all]. I would prefer to use rewrites to achieve this if possible, keeping CMS pages at (/pages/content-pages/[...all]) and rewriting per my last comment.

Thanks again!

[dphang]

Ah, I fixed the public file serving conflicting with the getStaticPaths with dynamic pages at root level.

For the rewrites issue, I see you are using Next.js 9.4.4 and @sls-next/serverless-component 1.17.0

OS/Environment: Mac
@sls-next/serverless-component version: 1.17.0
next: 9.4.4

Rewrites are supported only in Next.js 9.5 as per https://nextjs.org/docs/api-reference/next.config.js/rewrites, and you also need to upgrade @sls-next/serverless-component to the 1.18 alphas, where rewrites are implemented. Could you please upgrade to the latest 1.18.0-alpha.7 and see if that works for you. It will have rewrites implementation and also the fix I mentioned for getStaticPaths.

[patricktyndall]

Ah, I had upgraded to Next.js 9.5.5 and @sls-next/[email protected] for the rewrites approach (Sorry I didn't mention that in my original comment). The most recent comment was reported using this environment as well.

I just tried with @sls-next/[email protected] and I'm seeing the same issue and the same logs.

I can also comment in the rewrites RFC or open a new issue.

[dphang]

No worries... please open a separate issue so it can be better tracked. Also please ensure you are pinning the latest alpha version correctly in serverless.yml (not package.json), as others have also faced 404s for redirects because of this gotcha (where they end up using latest tag in npm which is 1.17 and doesn't have the logic to do redirects/rewrites).

If this is not the case, please open the issue and include your routes-manifest.json and the generated index.js (for both default and api handler folders in .serverless_nextjs folder) so I can debug further what is happening. The rewrites logic in the handler uses the rewrites defined in routes-manifest.json (which is generated by Next.js build) to do the rewrite.

I think this issue by itself should be resolved. Thanks!

[dphang]

Also with this rewrite config you mentioned:

return [{
	source: '/:path*',
	destination: '/:path*',
},{
	source: '/:path*',
	destination: '/content-pages/:path*',
}];

Just wondering if 1st rewrite is correct? Because the way it's implemented in this component, it will only match the 1st one and then the 2nd one is ignored. Let me test on Next dev server to see the behavior.

EDIT: on Next dev server it's also the same behavior, the 2nd rewrite is never matched since the 1st one matches it.

[dphang]

Actually, also need to handle the SSR page breaking, it looks like it also interferes with the dynamic page. I'll make a fix today.

[patricktyndall]

Thanks for checking, I have indeed been updating via serverless config.

Just wondering if 1st rewrite is correct?

That rewrite is from the next docs. The first one is there so SSG routes are checked first before the rewrite is done.

the 2nd rewrite is never matched since the 1st one matches it

Unsure what you're seeing on devserver, but I'm able to access pages using the second rewrite. Removing it causes 404s on pages that were working.

Removing the first also has some additional effects when using serverless-nextjs:

• Other matching static routes 503 (which in this case is everything)
• /public serving breaks again

(@sls-next/[email protected])

[dphang]

I see, I added rewrites to the e2e-test next app here:

module.exports = {
  async rewrites() {
    return [
      {
        source: "/:path*",
        destination: "/:path*"
      },
      {
        source: "/:path*",
        destination: "/ssg-page"
      }
    ];
  }
};

When I added the first rewrite, the next dev server is only matching the 1st rewrite for me, hence in that app it goes to a catch all page I defined at the root. If I remove the first one, then any path is rewritten to ssg-page and it's served correctly - an SSG page. This seemed to be same behavior on both Next.js dev server and serverless-next.js. Although this is Next.js 9.5.5, not sure if something changed with the later versions.

Anyway, so I thought that first no-op rewrite is only needed when you are proxying to another domain, which I hadn't implemented yet. But the way it's implemented now, it doesn't know about this special no-op rewrite; the first rewrite will get matched. I think there are a few things to fix:

1. We need to make sure we route existing SSR/SSG pages correctly when dynamic routes / catch-all at same level as those is present. I thought I fixed the public files / SSG pages, but SSR pages like you mentioned are broken (will fix it today).
2. For rewrites, I think there is some mismatch between this implementation and Next.js's. I will look into fixing the gap.
3. I'm not sure why public file serving is still breaking. Could you please check CloudWatch logs for any error responses and add them as well (and for the other broken routes, if they are now different?)?

BTW, do you have a minimal repro you can create and share? It will make it easier to debug and fix the specific scenario(s) you have in mind. Since in this case it looks like multiple things (catch-all, public files, SSR/SSG pages, rewrites) are interacting with each other, it may be hard for me to repro just via descriptions.

Thanks!

[patricktyndall]

Yep I can create a repo.

BTW - what's the best way to tear down resources after running the serverless command? This would help me deliver repo to you more quickly.

On the scenario you described:

• I think that catch-all at root is likely to be changing your results from mine (I'm not using this as I experience the rewrites issue)

My cloudwatch logs for broken pages are full of these two events:

[dphang]

@patricktyndall I don't use this and didn't implement it so I'm not completely sure how it works, but I thought there is a serverless remove command that will tear down the existing resources?

Thanks for providing the logs, I think that means the S3 key requested from origin request handler does not exist in the bucket itself. Hopefully the logs have more details on what the key value is or we can add more logging to provide a more helpful message - or I can just debug once I get a minimal repro.

I don't actually use the catch-all in my real apps so I probably have limited experience with all its nuances - so a minimal repro will help a lot! Thanks.

[dphang]

Have fixed the SSR/SSG pages conflict in: #685

[patricktyndall]

On the rewrites issue, I'll get a repo together this weekend.

---

Revisiting the original issue and looking your recent releases, I think it's broken when multiple catch-alls are involved. I have the following setup:

• no rewrites
• SSG catch-all at /pages/[...all] (using getStaticPaths/Props)
• SSR catch-all at /pages/products/[...all] (using getInitialProps)
• @sls-next/[email protected]

Now I get 404s when I try to visit /products/dynamic-product-slug. Other pages issues and public/ serving seem to be fixed though.

[patricktyndall]

@dphang created new issue and reproduction for rewrites issue: #696

[himynameistimli]

Hey just wanted to tag onto this issue. Also facing the dynamic SSG page conflicts with SSR pages.

I've put up a demo with v1.17.0 https://d2jucf2wqkn40w.cloudfront.net. I'll check to see if v1.18.0-alpha.14 helps.

Repo: https://github.com/himynameistimli/nextjs-serverless-503-cloudfront

[dphang]

@himynameistimli yes please try the latest alphas, there are fixes with conflicts with existing nonDynamic routes that were fixed

[himynameistimli]

hey @dphang yes, just updated the test repo and demo to @sls-next/[email protected].

I can confirm that the SSR static page works, but the dynamic SSR pages still do not work after refreshing the page. Returns a 404.

[dphang]

Thanks, let me take a look at the repro later this week.

[dphang]

@himynameistimli from repro you shared it seems the dynamic SSR page it is now working? 404s are cached by CloudFront/browser for some time I think, perhaps it is working from me since I'm hitting a different edge node (from Seattle).

[himynameistimli]

Hey @dphang my bad, was testing out a fix I made to lambda-at-edge, which I deployed to the demo.

https://github.com/himynameistimli/serverless-next.js/blob/6c966fdc6ed0b6909b164126cd58e8d5c20c5799/packages/libs/lambda-at-edge/src/default-handler.ts#L576-L583

It fixes it for my specific use case, but that's because I don't make use of fallbacks for SSG pages. I copied the repro manifests into a fiddle to see what was happening. https://jsfiddle.net/sbpxqeft/. hasFallbackForUri will return a different prerendered dynamic route (for an SSG page) if it has a more generalized regex that also happens to to work with a dynamic SSR route.

So I essentially told it to ignore the fallback if there is a dynamic SSR route that also matches. I don't know if this is generally right, but I think that there should be some check between the manifest and the prerenderManifest to make sure that they are talking about the same route.

I will restore the demo back to the latest beta. Sorry for the confusion.

[dphang]

Yea, I think it sounds like right direction but now that I think about it, I think we are mixing the fallback logic with the general routing logic...

Using routes-manifest.json is right way for routing as it has all of routes whereas prerender-manifest is just all the pre-rendered at build time routes.

From Next.js priority is as follows:

Predefined routes > Dynamic routes > Catch-all routes. I think all of this data is in the build manifest.

So we may need to rework the logic a bit: if it's matching one of them try to figure out page to render: either it's prerendered (e.g SSG from getStaticPaths), needs SSR (renders JS file) or has fallback

[patricktyndall]

Why is this closed? When I add a catch-all at /pages/[...all] I'm having requests for /_next/data being handled by getStaticProps, and then rendered on the server

[dphang]

@patricktyndall it was closed automatically by the merging of the linked PR, which I assumed fixed all the latest issues. Could you please add more details for the latest bug?

Is it still the same catch-all page with getStaticPaths and fallback = true? I thought I had added and verified tests for this, but let me check the behavior again.

[patricktyndall]

@dphang here's reproduction: https://github.com/patricktyndall/sls-next-bug-reproduction/tree/catchall-bug (note the branch)

• Deploy
• Visit homepage (/)
• Open devtools, look at the request for /_next/data/.../dynamic-1.json. Instead of returning the prebuilt JSON file for the page's props, it runs getStaticProps during the request for the json file. In other words, /_next/data requests are being handled by /pages/[...all].
  • This means that navigating to /dynamic-1 directly gives a different result than navigating there via a <Link>
• This behavior is broken regardless of fallback: true or fallback: false. However changing that does change the resulting JSON slightly.

(note also there are no rewrites involved here)

[darren-charterindex]

Possibly related to: #967

[dphang]

I think some of the latest changes may have fixed this as the routing precedence was fixed by @jvarho. Can you try in latest one and see if it works? Then I will close it if it is fixed.

I will close, feel free to comment or open a new issue in case it is still a problem

[wasurocks]

Hi, I'm still having this issue. It seems my favicon, which is served from the public folder, loads incorrectly (it loads a URL relative to my catch-all route). The favicon is being loaded in the head of my custom _document.tsx page. As for the catch-all route, it's just products/[...slug] and is using getStaticProps and getStaticPaths. Any suggestion would be appreciated :)

[wasurocks]

Hi, I'm still having this issue. It seems my favicon, which is served from the public folder, loads incorrectly (it loads a URL relative to my catch-all route). The favicon is being loaded in the head of my custom _document.tsx page. As for the catch-all route, it's just products/[...slug] and is using getStaticProps and getStaticPaths. Any suggestion would be appreciated :)

Okay so it's an error on my part and I have to apologize. The docs have to be followed strictly

The mistake I made was I referenced my favicon which was located in public/assets/favicon.ico by assets/favicon.ico which works for non-catch-all routes but not for catch-all routes. The correct way to write it would be /assets/favicon.ico. I hope whoever sees this finds it useful!