Skip to content
This repository has been archived by the owner on Feb 10, 2025. It is now read-only.

Routing too many redirects when using the config.json file generated by the Vercel adapter #418

Closed
1 task
yuhangch opened this issue Oct 9, 2024 · 23 comments · Fixed by #525
Closed
1 task
Assignees

Comments

@yuhangch
Copy link

yuhangch commented Oct 9, 2024

Astro Info

Astro                    v4.15.12
Node                     v18.20.4
System                   Windows (x64)
Package Manager          unknown
Output                   server
Adapter                  @astrojs/vercel/serverless
Integrations             unocss
                         astro-expressive-code

Describe the Bug

Configuration:

  1. Set trailingSlash: 'always' in astro.config.mjs
  2. Set build: { format: 'directory' }

Problem:
When visiting the /docs/ route, I encounter a "Redirected you too many times" error.

Details:

  • The directory structure is as follows:

    pages
    ├───docs
    │   ├───index.astro
    │   ├───[...slug].astro
    
  • The config.json file generated by the adapter looks like this:

    {
      "version": 3,
      "routes": [
        {
          "src": "/docs",
          "headers": {
            "Location": "/docs/"
          },
          "status": 308
        },
        {
          "src": "/docs/(?:\\/(.*?))?",
          "headers": {
            "Location": "/docs/$1/"
          },
          "status": 308
        }
      ]
    }

Issue:

  • The /docs/ route appears to redirect indefinitely, It seems the redirection rules are creating a loop for this route.

When delete trailingSlash: 'always' in astro.config.mjs, the redirect in config.json was not be generated, and /docs/ works well.

What's the expected result?

The /docs/ route should resolve correctly without an infinite redirect loop with trailingSlash: 'always' config.

Link to Minimal Reproducible Example

http://yuhang-ch-git-loop-redirect-yuhang-chens-projects.vercel.app/docs/

Participation

  • I am willing to submit a pull request for this issue.
@ematipico ematipico added the needs repro Issue needs a reproduction label Oct 9, 2024
@ematipico
Copy link
Member

@yuhangch the link you provided isn't a valid reproduction

@yuhangch
Copy link
Author

yuhangch commented Oct 9, 2024

Sorry, I just disable the Vercel Authentication

There is a capture:

20241009_193142.mp4

@ematipico
Copy link
Member

@yuhangch we need a reproduction which we can debug. Please the stackblitz URL provided

@yuhangch
Copy link
Author

Okay got it. I can't completely reproduce the issue on Stackblitz because the behavior is different on the dev server compared to when deployed with the Vercel adapter. Below is the simplest code. I have still deployed it to a sample project on Vercel.


@ematipico
Copy link
Member

It's okay, we can deploy it ourselves on Vercel

@ematipico ematipico added needs triage Issue needs to be triaged and removed needs repro Issue needs a reproduction labels Oct 10, 2024
@bgentry
Copy link

bgentry commented Oct 25, 2024

I believe I just encountered this same issue when using trailingSlash: 'never'. The regression appears to be introduced as of @astrojs/vercel v7.8.2.

I have a route served by a rest param for an SSR page at /path/[...slug].astro. This route is rendering some external content which is prerendered with relative links, so to make that work correctly I'm using trailing slashes only on this path prefix via redirects within the page's JS:

---
import PackagePageLayout from '@/layouts/pkg/PackagePageLayout.astro';
import { getEntry } from 'astro:content';

const { slug } = Astro.params;
if (!slug) {
  return new Response('Not found', { status: 404 });
}

if (!slug.endsWith('/')) {
  // Permanent redirect to the canonical URL with trailing slash:
  const url = new URL(Astro.request.url);
  url.pathname = `${url.pathname}/`;
  return Astro.redirect(url, 301);
}

// fetch content and render ...
---
<html />

With the @astrojs/vercel adapter v7.8.1, this scheme works just fine. Paths under this prefix are redirected to have a trailing slash, both locally and on Vercel. However, as soon as I upgrade to v7.8.2, this no longer works and I instead see an infinite redirect loop between the paths with and without the trailing slash. I'm specifically getting a 308 Permanent Redirect from the trailing slash path back to the one without.

Locally, this still works fine, it's only once deploying to vercel that it breaks.

@artt
Copy link

artt commented Dec 7, 2024

Just bumped into this as well. Created a reproducible repo here if it helps: https://github.com/artt/trailingslash-vercel

@yuhangch @bgentry did you find any workaround?

@bgentry
Copy link

bgentry commented Dec 7, 2024

No, my workaround has been to pin the version from before the breakage and hope somebody takes action on this reported issue 😂🤞🏻

@artt
Copy link

artt commented Dec 8, 2024

For some reason 7.8.1 still doesn't work for me. https://github.com/artt/trailingslash-vercel/tree/v7.8.1

@bgentry
Copy link

bgentry commented Jan 2, 2025

For my specific issue, I have narrowed it down to #373 / 5b802a4 that introduced this breakage (cc @hrabiel). If I use patch-package to apply this diff to revert the meat of that commit, my 308 redirect issue goes away altogether.

diff --git a/node_modules/@astrojs/vercel/dist/lib/redirects.js b/node_modules/@astrojs/vercel/dist/lib/redirects.js
index 47b2621..00d4857 100644
--- a/node_modules/@astrojs/vercel/dist/lib/redirects.js
+++ b/node_modules/@astrojs/vercel/dist/lib/redirects.js
@@ -28,31 +28,25 @@ function getParts(part, file) {
 // 2022-04-26
 function getMatchPattern(segments) {
     return segments
-        .map((segment, segmentIndex) => {
-        return segment.length === 1 && segment[0].spread
-            ? '(?:\\/(.*?))?'
-            : // Omit leading slash if segment is a spread.
-                // This is handled using a regex in Astro core.
-                // To avoid complex data massaging, we handle in-place here.
-                (segmentIndex === 0 ? '' : '/') +
-                    segment
-                        .map((part) => {
+        .map((segment) => {
+            return segment[0].spread
+                ? '(?:\\/(.*?))?'
+                : segment
+                    .map(((part) => {
                         if (part)
-                            return part.spread
-                                ? '(.*?)'
-                                : part.dynamic
-                                    ? '([^/]+?)'
-                                    : part.content
-                                        .normalize()
-                                        .replace(/\?/g, '%3F')
-                                        .replace(/#/g, '%23')
-                                        .replace(/%5B/g, '[')
-                                        .replace(/%5D/g, ']')
-                                        .replace(/[*+?^${}()|[\]\\]/g, '\\$&');
-                    })
-                        .join('');
-    })
-        .join('');
+                            return part.dynamic
+                                ? '([^/]+?)'
+                                : part.content
+                                    .normalize()
+                                    .replace(/\?/g, '%3F')
+                                    .replace(/#/g, '%23')
+                                    .replace(/%5B/g, '[')
+                                    .replace(/%5D/g, ']')
+                                    .replace(/[*+?^${}()|[\]\\]/g, '\\$&');
+                    }))
+                    .join('');
+        })
+        .join('/');
 }
 function getReplacePattern(segments) {
     let n = 0;

@ascorbic
Copy link
Contributor

Can you install @astrojs/vercel@experimental--vercel-routing and see if it fixes it please?

@ascorbic ascorbic self-assigned this Jan 29, 2025
@ascorbic ascorbic removed the needs triage Issue needs to be triaged label Jan 29, 2025
@bgentry
Copy link

bgentry commented Jan 29, 2025

@ascorbic Just tried it, does not fix my issue. Hitting the path with the trailing slash gives me a 308 to the same route without the trailing slash. The content of this route with rest param is in my comment above: #418 (comment)

Running locally without vercel, this works just fine and I'm allowed to render the routes with the trailing slash (despite trailingSlash: 'never' in my astro.config.mjs). But when deployed on Vercel, there's a redirect loop due to the 308 which happens before my route is ever executed:

HTTP/2 308 
cache-control: public, max-age=0, must-revalidate
content-type: text/html
date: Wed, 29 Jan 2025 15:59:35 GMT
location: /mypath/subroute
refresh: 0;url=/mypath/subroute
server: Vercel
strict-transport-security: max-age=63072000; includeSubDomains; preload
x-robots-tag: noindex
x-vercel-id: cle1::xhbk9-1738166375829-07bb4fd918e0

@ascorbic
Copy link
Contributor

@bgentry could you share a minimal reproduction so I can take a look?

@ascorbic
Copy link
Contributor

@artt your repro seems to work for me with the experimental package: https://trailingslash-vercel-six.vercel.app/

@bgentry
Copy link

bgentry commented Jan 29, 2025

@ascorbic sure, here's a simple repro created with these steps:

  1. Create a new Astro app
  2. Install @astrojs/vercel
  3. Add sub/[...slug].astro page with a simplified version of my content above
  4. Set trailingSlash: 'never' in astro.config.mjs.

It's deployed on Vercel, try to visit /sub/foo/ here and you'll see the redirect loop in action.

Note that it appears step 4 seems to be a factor here. Without it, I get some redirect loop behaviors locally as well due to the redirect within my route.

@ascorbic
Copy link
Contributor

@bgentry that repro is using the published adapter version. Have you tried it with the experimental version that I shared above?

@bgentry
Copy link

bgentry commented Jan 29, 2025

@ascorbic sorry, wasn't clear which version you wanted on the repro. Yes I have tried the experimental version, it did not fix the issue on my real app and it doesn't fix it on the repro either. I pushed the updated version using the experimental @astrojs/vercel@experimental--vercel-routing, same behavior. It's live now and the repo was updated too.

@ascorbic
Copy link
Contributor

ascorbic commented Jan 30, 2025

@bgentry it's doing a redirect loop because you have trailingSlash: "never", but are then returning a manual redirect inside the page that redirects it back to the page with the trailing slash.

https://github.com/bgentry/astro-vercel-redirect-loop-demo/blob/master/src/pages/sub/%5B...slug%5D.astro#L7-L12

I removed that and it's working fine: https://astro-vercel-redirect-loop-demo-ten.vercel.app/sub/foo

@ascorbic
Copy link
Contributor

The fix is in @astrojs/[email protected]

@bgentry
Copy link

bgentry commented Jan 30, 2025

@ascorbic that is intentional. It works fine without the Vercel adapter and it worked fine with the adapter up until #373, which broke this behavior.

The reason I need the trailing slash on these routes is that I’m serving up pre rendered HTML content with relative links. See the real deployment here: https://riverqueue.com/pkg/riverpro/latest/riverpro/

Again, this works fine everywhere (including locally) except when deployed with the Vercel adapter after #373 broke it.

@ascorbic
Copy link
Contributor

If you need the trailing slash, why are you setting trailingSlash: "never"? By setting trailingSlash: "never" you're telling it to never use trailing slashes, but then you're using the redirect to add trailing slashes. So you are manually redirecting from /sub/foo to /sub/foo/, which Vercel then redirects back to /sub/foo because of the trailing slash setting and so on.

@bgentry
Copy link

bgentry commented Jan 30, 2025

@ascorbic the rest of my site relies on trailingSlash: "never" and predates the static generated pkg doc section that uses trailing slashes. The generated doc pages are served up by a dynamic route in server-rendered mode, and this has worked well because the trailingSlash setting does not seem to take precedence over what's decided by the dynamic route (at least not in server rendered mode).

It seems that as of #373 @astrojs/vercel is using this setting to 308 redirect on all requests with a trailing slash, whether or not they are dynamically rendered and without giving the dynamic route an opportunity to decide what it wants to do. Again, this is different than what happened previously and it is not what Astro itself does—so it is a breaking change specific to the Vercel adapter.

The version that's deployed live is using the latest @astrojs/vercel release but with a custom patch that reverts the entirety of #373. This behaves exactly as it did prior to that release breaking the behavior.

@ascorbic
Copy link
Contributor

This was a bug before (and is this bug is a side effect of it). It was supposed to work like it does currently. If you want it to not be "always" you can set it to "ignore"

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

Successfully merging a pull request may close this issue.

5 participants