-
Notifications
You must be signed in to change notification settings - Fork 276
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
Redirect middleware plugin causes MIDDLEWARE_INVOCATION_TIMEOUT error when ANY request has large number of query params #2004
Comments
Hi @chrissnyder2337 thanks for the detailed submission! |
As a temporary workaround, but not ideal, to avoid people from getting 504 MIDDLEWARE_INVOCATION_TIMEOUT errors whenever requests have many query parameters, disable the redirect plugin from running at all if there are more than NOTE That this workaround prevents redirects from happening at all if the url the person is visiting with has many params (like utm and marketing parameters) in import { NextRequest, NextResponse } from 'next/server';
import { RedirectsMiddleware } from '@sitecore-jss/sitecore-jss-nextjs/middleware';
import { MiddlewarePlugin } from '..';
import { debug } from '@sitecore-jss/sitecore-jss-nextjs/middleware';
import { siteResolver } from 'lib/site-resolver';
import clientFactory from 'lib/graphql-client-factory';
class RedirectsPlugin implements MiddlewarePlugin {
private redirectsMiddleware: RedirectsMiddleware;
order = 0;
constructor() {
this.redirectsMiddleware = new RedirectsMiddleware({
// Client factory implementation
clientFactory,
// These are all the locales you support in your application.
// These should match those in your next.config.js (i18n.locales).
locales: ['en', 'es'],
// This function determines if a route should be excluded from RedirectsMiddleware.
// Certain paths are ignored by default (e.g. Next.js API routes), but you may wish to exclude more.
// This is an important performance consideration since Next.js Edge middleware runs on every request.
excludeRoute: () => false,
// This function determines if the middleware should be turned off.
// By default, it is disabled while in development mode.
disabled: () => process.env.NODE_ENV === 'development',
// Site resolver implementation
siteResolver,
});
}
/**
* exec async method - to find coincidence in url.pathname and redirects of site
* @param req<NextRequest>
* @returns Promise<NextResponse>
*/
async exec(req: NextRequest, res?: NextResponse): Promise<NextResponse> {
// This is a temporary workaround to bypass this plugin when there are more than 5ish query parameters which causes a MIDDLEWARE_INVOCATION_TIMEOUT error.
// This should be removed once this is fixed in sitecore JSS.
if ([...req.nextUrl.searchParams.entries()].length > 5) {
debug.redirects(
'(temp workaround) bypassing JSS redirect plugin due to more than 5 search params'
);
return res || NextResponse.next();
}
return this.redirectsMiddleware.getHandler()(req, res);
}
}
export const redirectsPlugin = new RedirectsPlugin(); |
Describe the Bug
Redirect middleware plugin causes MIDDLEWARE_INVOCATION_TIMEOUT errors when large number of query parameters are included in the request's url. 8 or 9ish.
Having this many query parameters is quite normal in modern web applications, and the number of query parameters should not have any performance impact on every single request.
Possible Issue:
I think the
isPermutedQueryMatch
defined in https://github.com/Sitecore/jss/blob/dev/packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts is causing issues. TheisPermutedQueryMatch
method generates all permutations of query parameters to test them against regex patterns. When the query string contains many parameters, the number of permutations grows factorially (n!
). This can result in an enormous number of permutations and significantly slow down the processing.Example:
5
parameters, there are120
permutations.10
parameters, there are3,628,800
permutations.To Reproduce
a
redirects to/b
works fine.Expected Behavior
The redirect plugin does not cause excessive computation and does not cause MIDDLEWARE_INVOCATION_TIMEOUT errors.
Possible Fix
Handle query parameters better and fix the logic and performance of the redirect middleware plugin.
Provide environment information
The text was updated successfully, but these errors were encountered: