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

Interest in testing with nextjs? #227

Open
jshthornton opened this issue Apr 3, 2022 · 17 comments
Open

Interest in testing with nextjs? #227

jshthornton opened this issue Apr 3, 2022 · 17 comments

Comments

@jshthornton
Copy link

I've managed to get this working to provide Lambda powered nextjs apps.
Next has deprecated their serverless target so I needed another way to do it.

I'm happy to spin up a fork and write some tests to make sure it's a first class supported framework.

Thoughts?

@dougmoscrop
Copy link
Owner

Sure! We're using serverless-http as part of Serverless Cloud and Next 12 has worked in all the tests we've done, but we did add some things inside the Runtime to make it work - anything you feel would be beneficial here is welcome.

@MrAdamCook
Copy link

I'd absolutely love to see this working with NextJS, just been trying to tackle that myself 😅

@fastner
Copy link
Contributor

fastner commented Apr 7, 2022

I've tested serverless-http in combination with the nextConfig.experimental.outputStandalone and it is mostly working. One finding is that Uint8Array is not supported. I've added a PR for that: #232.

@anthonyroach
Copy link
Contributor

anthonyroach commented Jul 22, 2022

I've tested the latest version of serverless-http with nextjs nextConfig.experimental.outputStandalone extensively and it works well. Also it looks like the nextjs standalone output type is no longer going to be experimental once next.js v12.2.3 releases. So it seems like what's left is to add an automated test for next.js to this repo and then document in the README.md that next.js is supported.

In case it is helpful to anyone, here's the server.ts I'm using (replaces server.js that the standalone output produces):

import NextServer from 'next/dist/server/next-server';
import path from 'path';
import serverless from 'serverless-http';
import requiredServerFiles from './.next/required-server-files.json';

const nextServer = new NextServer({
  hostname: 'localhost',
  port: 3000,
  dir: path.join(__dirname),
  dev: false,
  conf: {
    ...requiredServerFiles.config,
    // Assume this lambda sits behind a CF distro that 
    // will compress all the things:
    compress: false,
  },
});

export const handler = serverless(nextServer.getRequestHandler(), {
  // enable binary support for all content types:
  binary: ['*/*'],
});

@anthonyroach
Copy link
Contributor

anthonyroach commented Aug 9, 2022

I did end up running into one problem with API routes when there is a request body. Next.js has a built in body parser middleware that won't parse the body if there's already a body field on the request, and serverless-http adds body field to the request. So I had to add a request modifier to remove the body field like so:

return serverless(nextServer.getRequestHandler(), {
    // enable binary support for all content types:
    binary: ['*/*'],
    request: (request) => {
      // nextjs has a built in body parser middleware that won't parse
      // the body if there's already a body field on the request,
      // and serverless-http adds body field to the request, so
      // remove it:
      delete request.body;
    },
  });

Here's the offending line in nextjs:

https://github.com/vercel/next.js/blob/canary/packages/next/server/api-utils/node.ts#L206

@jinqian1998
Copy link

Thanks a lot, now the nextJS has already regard the "standalone" as a formally option. Hope it will come to our example soon XD.

Follow @anthonyroach 's code, I created this, just move all mjs usage to cjs usage.

const NextServer = require("next/dist/server/next-server").default
const path = require("path")
const serverless = require("serverless-http")
const requiredServerFiles = require("./.next/required-server-files.json")

const nextServer = new NextServer({
  hostname: "localhost",
  port: 3000,
  dir: path.join(__dirname),
  dev: false,
  conf: {
    ...requiredServerFiles.config,
    // Assume this lambda sits behind a CF distro that
    // will compress all the things:
    compress: false,
  },
});

exports.handler = serverless(nextServer.getRequestHandler(), {
  // enable binary support for all content types:
  binary: ["*/*"],
  request: (request) => {
    // nextjs has a built in body parser middleware that won't parse
    // the body if there's already a body field on the request,
    // and serverless-http adds body field to the request, so
    // remove it:
    delete request.body;
  },
});

@kerbe
Copy link

kerbe commented Nov 5, 2022

I've been able to get my NextJS 12.3.2 project mostly to work with AWS Lambda.

I have tried adding NextAuth.js authentication (with AWS Cognito backend). It works really fine on localhost, but once deployed to AWS Lambda, it has given headache.

Initially when I pressed signIn() button, I only got vague JSON parse error. Once I tried adding that request.body delete line, things move forward.

Currently problem is that final redirect to AWS Cognito hosted login page doesn't happen. Currently there is no errors visible anywhere at all.

Currently code looks like this:

// server.ts
process.chdir(__dirname);

import { NextConfig } from "next";
import NextServer, { Options } from "next/dist/server/next-server";
import serverless from "serverless-http";
import path from "path";
// @ts-ignore
import { config } from "./.next/required-server-files.json";
import { ServerResponse } from "http";

const nextConfig: Options = {
    hostname: "localhost",
    port: 3000,
    dir: path.join(__dirname),
    dev: false,
    customServer: false,
    conf: {
        ...(config as NextConfig),
        compress: false
    },
};

const getErrMessage = (e: any) => ({ message: 'Server failed to respond.', details: e });

const nextServer = new NextServer(nextConfig).getRequestHandler();

export const handler = serverless(async (req: any, res: ServerResponse) => {
    await nextServer(req, res).catch((e) => {
        console.error(`NextJS request failed due to: `);
        console.error(e);

        res.setHeader('Content-Type', 'application/json');
        res.end(JSON.stringify(getErrMessage(e), null, 3));
    })
}, {
    binary: ["*/*"],
    request: (request: any) => {
        delete request.body;
    }
});

Any pointers how to further debug this problem? Where to add logging/debugging?

@johnforte
Copy link

// nextjs has a built in body parser middleware that won't parse
      // the body if there's already a body field on the request,
      // and serverless-http adds body field to the request, so
      // remove it:

Thank you @anthonyroach, for the tidbit that saved me some deep searching. Just curious why you use standalone? Just using the nextjs request handler from the normal package works perfectly fine for me. I am also using lambda with docker contains, so I have a bit more space in the uploaded package rather than packing with a zip.

const serverless = require('serverless-http');
const next = require('next');
const nextHandler = next({dev: false});

module.exports.handler = serverless(nextHandler.getRequestHandler(), {
  binary: ['*/*'],
  request: (request) => {
    delete request.body;
  },
});

@anthonyroach
Copy link
Contributor

@johnforte You're welcome, I'm glad it saved you some time.

I decided to use standalone just so the lambda zip was smaller. I tried using non-standalone and bundling with the CDK node lambda construct instead, and couldn't get that to work.

@vs-cristian
Copy link

@anthonyroach, have you managed to run it with nextjs 13? There's an open issue regarding nextjs throwing an error when running standalone. I'm encountering the it too.

vercel/next.js#49169

@johnforte
Copy link

@anthonyroach interesting, I have switched to lambda dockers and that works fine for me.

@jaredhm
Copy link

jaredhm commented Dec 13, 2023

Hate to revive a thread that seems to have lived and died peacefully, but after borrowing from this approach, I've been seeing 404s on _next/data routes. I've confirmed that the 404s are originating from next and not some downstream layer (e.g. API Gateway) through the presence of x-powered-by: Next.js in the response, as well as a custom header I'm adding in ServerlessHttp.Options.response, which contains the invocation ID passed to the Lambda (i.e. the one described here)

Using node@16.16.0, serverless-http@3.1.0, next@13.5.4. Any other setup information I can provide?

@kerbe
Copy link

kerbe commented Dec 13, 2023

Using node@16.16.0, serverless-http@3.1.0, next@13.5.4. Any other setup information I can provide?

Isn't node@18 or even node@20 minimum requirement? At least if I try to start my nextjs dev environment, it cries:
You are using Node.js 16.16.0. For Next.js, Node.js version >= v18.17.0 is required.

I'm currently working with NextJS 14.0.3, using node v20.10.0 and I haven't yet ran into issues. I don't use _next/data myself right now, so could be that I have similar problem, but if you'd be able to throw some minimum reproducible repo together, I can give it a go how it goes with my settings & tweakings @jaredhm .

I'm fairly confident that this is viable approach, and worth trying to get working.

@jaredhm
Copy link

jaredhm commented Dec 13, 2023

@kerbe thanks for the quick reply - I'll try and get a minimal setup going that reproduces the errors

@jaredhm
Copy link

jaredhm commented Dec 14, 2023

hello again @kerbe, I've created a repo here: https://github.com/jaredhm/serverless-http_issues_227 and deployed the application to AWS (link). I'm somewhat surprised to report that the issues persists!

Screen.Recording.2023-12-14.at.2.21.21.PM.mov

(I don't think that the favicon 404s are related - I probably just goofed up some step of the package process)

@Russ93
Copy link

Russ93 commented Feb 1, 2024

@jaredhm I found the issue and found a monkey patch for it.

Ref: https://github.com/vercel/next.js/blob/canary/packages/next/src/server/pipe-readable.ts#L70
Has to deal with ^ this line in NextJS's code. It should be expecting ok === false but "serverless-http" is returning undefined

So for a quick fix in your createServer((req,res) => { ... })

Put this snippet

// NextJs will think there are back pressure issues
// If res.write doesn't return true
res.ogWrite = res.write;
res.write = function (...args) {
  res.ogWrite(...args);
  return true;
};

Like so

createServer(async (req, res) => {


      // NextJs will think there are back pressure issues
      // If res.write doesn't return true
      res.ogWrite = res.write;
      res.write = function (...args) {
        res.ogWrite(...args);
        return true;
      };


      await nextServer.handle(req, res, parsedUrl);

});

@andrewmota
Copy link

Hello everyone, any updates on 404s when using serverless-http?

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

No branches or pull requests