Bug - API Routes do not handle query parameters.

[mattdell]

The merge of #99 and #117 has given us support for API Routes in NextJS 🎉

However, I have noticed a bug when sending query parameters.

Here is a copy of my endpoint (apologies for the poor naming conventions but this is a WIP). It simply takes a relative path request and proxies to the fully qualified path on the server. Whether or not this is a best practice at this point is 🤷‍♂

// pages/api/next/[path]/[endpoint].ts

import { NextApiRequest, NextApiResponse } from 'next';
import request from 'axios';
import qs from 'query-string';

export default async (req: NextApiRequest, res: NextApiResponse) => {
  const {
    query: { path, endpoint, ...rest },
  } = req;

  const queryString = rest ? `?${qs.stringify(rest)}` : '';
  const url = `${process.env.HOST}/${path}/${endpoint}${queryString}`;

  try {
    const response = await request.get(url);
    res.status(200).json(response.data);
  } catch (error) {
    res.status(500).json({ error: error.message});
  }
};

And I call it with this endpoint:

/api/next/classic-api/news-listing.asp?catId=26&skip=9&take=8

When I run this locally, the NextJS convention is that

• path resolves to classic-api
• endpoint resolves to news-listing.asp
• queryString resolves to ?catId=26&skip=9&take=8

When I deploy this to AWS, API Gateway resolves to this

• path resolves to classic-api
• endpoint resolves to news-listing.asp?catId=26&skip=9&take=8&path=classic-api&endpoint=news-listing.asp
• queryString resolves to ?catId=26&skip=9&take=8

The problem seems to be how endpoint is being resolved in AWS.

Instead of forwarding on my request to

https://www.mydomain.com/classic-api/news-listing.asp?catId=26&skip=9&take=8

it instead forwards it on to

https://www.mydomain.com/classic-api/news-listing.asp?catId=26&skip=9&take=8&path=classic-api&endpoint=news-listing.asp?catId=26&skip=9&take=8

If anyone has any ideas of where to get started it would be much appreciated. Ultimately I want the plugin to behave in the same way as NextJS.

[mattdell]

@danielcondemarin - any chance this could be something within next-aws-lambda?

I see there's some query string manipulation in compatLayer.js

  if (hasQueryString) {
    req.url += `?${qs}`;
  }

https://github.com/danielcondemarin/serverless-nextjs-plugin/blob/master/packages/next-aws-lambda/lib/compatLayer.js#L38

[Deadleg]

I encountered this yesterday as well, there's actually a bug with nextjs which incorrectly matches on the last path segment. I'm working on a fix here https://github.com/Deadleg/next.js/commit/5bdcd698bd6aaf1ac1e0d53fd50b94636e17c458 but the long and short of it there is a regex in packages/next-server/lib/router/utils/route-regex.ts which matches on every non / character, which in turn matches on question marks and everything after.

Note that if you run next dev then it doesn't reproduce, but if you use next build and next start or serverless offline then you can reproduce it since the regex is only applied to the generated serverless functions.

You can apply the below patch which works around the issue, but breaks custom paths. I think if nextjs is fixed then we don't need to do anything in this project.

diff

diff --git a/packages/next-aws-lambda/lib/compatLayer.js b/packages/next-aws-lambda/lib/compatLayer.js
index 7d19fa6..213c7aa 100644
--- a/packages/next-aws-lambda/lib/compatLayer.js
+++ b/packages/next-aws-lambda/lib/compatLayer.js
@@ -10,8 +10,9 @@ const reqResMapper = (event, callback) => {
     multiValueHeaders: {}
   };
 
+
   const req = new Stream.Readable();
-  req.url = (event.requestContext.path || event.path || "").replace(
+  req.url = (event.path || "").replace(
     new RegExp("^/" + event.requestContext.stage),
     ""
   );
@@ -22,15 +23,15 @@ const reqResMapper = (event, callback) => {
     qs += queryString.stringify(event.multiValueQueryStringParameters);
   }
 
-  if (event.pathParameters) {
-    const pathParametersQs = queryString.stringify(event.pathParameters);
-
-    if (qs.length > 0) {
-      qs += `&${pathParametersQs}`;
-    } else {
-      qs += pathParametersQs;
-    }
-  }
+//  if (event.pathParameters) {
+//    const pathParametersQs = queryString.stringify(event.pathParameters);
+//
+//    if (qs.length > 0) {
+//      qs += `&${pathParametersQs}`;
+//    } else {
+//      qs += pathParametersQs;
+//    }
+//  }
 
   const hasQueryString = qs.length > 0;

[Deadleg]

You can work around it by not using a dynamic file name e.g. change pages/api/next/[path]/[endpoint].ts -> pages/api/next/[path][endpoint]/index.ts.

[Deadleg]

Ignore the work around, need to use the patch. Created pull request here vercel/next.js#8386

[mattdell]

Amazing detail @Deadleg - thanks for raising that PR!

[Deadleg]

Next.js 9.0.4 has just been released, from my testing APIs are working correctly.

[mattdell]

I've updated to Next 9.0.4 and I keep getting a 502 from all endpoints.

This one is from my index page at / but all Next pages have the same behavior.

Syntax error in module 'sls-next-build/index': SyntaxError
} catch {
^

SyntaxError: Unexpected token {
at createScript (vm.js:80:10)
at Object.runInThisContext (vm.js:139:10)
at Module._compile (module.js:616:28)
at Object.Module._extensions..js (module.js:663:10)
at Module.load (module.js:565:32)
at tryModuleLoad (module.js:505:12)
at Function.Module._load (module.js:497:3)
at Module.require (module.js:596:17)
at require (internal/module.js:11:18)
at Object.<anonymous> (/var/task/sls-next-build/index.js:2:16)

[Deadleg]

That doesn't look right, sls-next-build/index should have the contents of lambdaHandlerWithFactory in https://github.com/danielcondemarin/serverless-nextjs-plugin/blob/7c25b305739b134aff53c83007868022f46adaa8/packages/serverless-nextjs-plugin/lib/getFactoryHandlerCode.js. What's in that file on your lambda?

[mattdell]

It is...

  const page = require("./index.original.js");
  const handlerFactory = require("next-aws-lambda");

  module.exports.render = (event, context, callback) => {
    const handler = handlerFactory(page);
    handler(event, context, callback);
  };

[Deadleg]

Looks like a new feature in ES10 called optional catch binding is causing the syntax error. Upgrading node may fix the issue, I'm building and running with the latest available version on Lambda.

[mattdell]

Good catch! I was running Node 8 in AWS so I've bumped it to Node 10 in my serverless.yml and I've got a website again. :)

provider:
  name: aws
  runtime: nodejs8.10

provider:
  name: aws
  runtime: nodejs10.x