sst / open-next

Open source Next.js serverless adapter
https://open-next.js.org
MIT License
3.83k stars 115 forks source link

Re-enable nextjs compression or allow enabling it optionally #475

Open janus-reith opened 1 month ago

janus-reith commented 1 month ago

Although Cloudfront does brotli compression on its own, there are good reasons to have the nextjs lambda compress on its own.to avoid hitting the 6 MB Limit. In my case when using Payload CMS, a 10 MB file shrinks to ~400 kb using the nextjs gzip compression.

This comment mentions that compression which appareantly caused issues in earlier version actually seems fine now: https://github.com/sst/open-next/issues/35#issuecomment-1411131129

conico974 commented 1 month ago

We probably don't want to re enable this. First of all, it will only work for routes, not for middleware response or external rewrite. And it only does gzip compression.

The way to do this properly is probably with a custom wrapper https://open-next.js.org/config/custom_overrides#custom-wrapper

Something like that should do the trick

// customWrapper.ts
import streamingWrapper from 'open-next/wrappers/aws-lambda.js'
import {WrapperHandler} from 'open-next/types/open-next'

const handler : WrapperHandler = async (handler, converter) => {
  const defaultHandler = await streamingWrapper.wrapper(handler, converter)
  return async (event: APIGatewayProxyEventV2): Promise<APIGatewayProxyResultV2> => {
    const acceptHeaders = event.headers["accept-encoding"];
    const result = await defaultHandler(event)
    const compressedBody = // Here you compress with node zlib result.body
    return {
      ...result,
     headers: {
       ...result.headers,
      'content-encoding': 'br' // Or whatever
     },
    body: compressedBody
    }
  }
}

export default {
  wrapper: handler,
  name: "custom-aws-lambda",
  supportStreaming: false,
};
janus-reith commented 1 month ago

Thanks a lot for the reply, that seems like the way to go.

I'm using open-next with SST Ion currently, which uses open-next internally. Do you know by chance how I could add such a wrapper there? I'm still puzzling together some of the pieces and am unsure what's overridable and what not.

conico974 commented 1 month ago

You need a custom open-next.config.ts https://open-next.js.org/config. How to use it with a custom wrapper is the link i put in the previous comment. You can close the issue if you've managed to fix this

janus-reith commented 1 month ago

Thanks, just figured it out. I'm wondering though, if using the default gzip compression isn't a valid usecase aswell.

First of all, it will only work for routes, not for middleware response or external rewrite.

In my case, thats the only place where I'd need it and where it would have a notable effect. Would there be an issue with at at least keeping it disabled, but optionally allowing to enable it, either if specified explicitely in your next.config or with some separate flag?

Or do you mean middleware responses or external rewrites would break in case that would be enabled?

conico974 commented 1 month ago

It probably wouldn't break anything in most cases (it might still break things for earlier version of next). The thing that is not really good is that the same lambda will produce either a compressed response (for route) or an uncompressed one. I think the proper way to deal with this in OpenNext would be to either create an aws-lambda-compressed wrapper here or to add a flag to enable compression in the default lambda wrapper itself. This would allow us to properly support deflate, gzip and brotli compression instead of just gzip & deflate

janus-reith commented 1 month ago

I was able to get gzip compression working and it did solve my lambda response issues, thanks for the hints!

What’s left now is that there seems to be some issue with cookies being swallowed, at least that’s what I think is hapoening since signins to my app fail although there’s a successful response and no cookies are set.

Anz chance there are some known issues with this when a custom wrapper is used? From what I see, my wrapper leaves all existing headers as they are and only adds Content-Encoding and Content-Length

conico974 commented 1 month ago

This should have no effect at all, custom wrapper uses the exact same logic as integrated wrapper. The only thing that could cause some issue in a custom wrapper would be if you don't properly propagate the response. Lambda expect cookies to be set in a separate cookies field What version of Next and OpenNext are you using ? Did you look for logs in cloudwatch ?

janus-reith commented 1 month ago

What version of Next and OpenNext are you using ? Next 15.0.0-canary.58 currently and open-next 3.0.2

I just tried on a different app which uses Payload CMS aswell where Login / Cookies worked on the deployment to add the same wrapper for compression. That one has less complexity and therefore didn't scratch the 6 MB response limit yet. The same happens there, so.I could verify that it's indeed the custom wrapper causing it.

Sorry, this is going a bit beyond the scope of the original issue, but I believe it could also help other people running into response size limits.

This is what I'm using currently. I think I'm passing everything from the orignal response properly.

[...]
import defaultWrapper from 'open-next/wrappers/aws-lambda.js'
import zlib from 'node:zlib'

const handler: WrapperHandler = async (handler, converter) => {
  const defaultHandler = await defaultWrapper.wrapper(handler, converter)

  return async (event: AwsLambdaEvent): Promise<AwsLambdaReturn> => {
    const result = await defaultHandler(event)

    const bodyString = typeof result.body === 'string' ? result.body : JSON.stringify(result.body)

    const compressedBody = zlib.gzipSync(bodyString)

    const response = {
      ...result,
      body: compressedBody.toString('base64'),
      headers: {
        ...result.headers,
        'Content-Encoding': 'gzip',
        'Content-Length': compressedBody.length,
      },
      isBase64Encoded: true,
    }

    return converter.convertTo(response, event)
  }
}

export default {
  name: 'custom-aws-lambda',
  supportStreaming: false,
  wrapper: handler,
}
janus-reith commented 1 month ago

Another detail I just found when comparing the API route for login from a previous regular deployment that did not have the custom wrapper with the one that has it and is now missing the Set-Cookie header:

While the wrapped one now has a gzip encoding header, the regular doesn't have any. I expected the default behaviour of the Cloudfront distribution to compress with brotli.

This might be related, maybe it's also out of the scope of open-next: https://github.com/sst/ion/issues/723

conico974 commented 1 month ago

I don't see how just using a custom wrapper by itself could cause an issue. The code for importing the wrapper is this https://github.com/sst/open-next/blob/a43fa46e9aeaf18b3c68647d1ef2beca3b05f290/packages/open-next/src/core/resolve.ts#L30-L42 I'm pretty sure you don't need to run the converter again here, that's probably your issue, the defaultHandler is already returning converter.convertTo, and the default one is removing the set-cookie header. Just returing response should be enough

janus-reith commented 1 month ago

that's probably your issue, the defaultHandler is already returning converter.convertTo, and the default one is removing the set-cookie header.

It indeed was! Again, thanks a lot!

janus-reith commented 1 month ago

I'll keep this open since I believe adding compression could be more straightforward. Maybe some implementation that would align with the next.config compress option could be helpful to align closer with how nextjs behaves. It could check if compress is explicitly set to true in your next config and add compression to the lambda based on that (Although that also wouldn't reflect the original next behaviour 1:1 where it's applied by default.)

But of course feel free to close if you think otherwise.

Since the same app that failed due to the respose size worked on Vercel, I assume they also compress at the lambda level, or have some other limits with AWS than regular accounts.