sladg / nextjs-lambda

Lambda deployments for Nextjs12 & Nextjs13 (standalone). Easy CLI commands to get your standalone Next output to run in AWS Lambda (not @Edge)! Uses CDK in behind and produces code zips importable to Terraform, Serverless, Azure, etc.
MIT License
169 stars 19 forks source link

Unable to have an API route that returns binary data #56

Closed john-tipper closed 1 year ago

john-tipper commented 1 year ago

I want to have an API route that returns binary data - by way of example, let's say that there's an API route that generates a dynamic image /api/image/generate and where the content is returned by something looking like this:

export default async (req: NextApiRequest, res: NextApiResponse) => {

...
  // buffer contains the image of type defined by mime, is of type Buffer
  res.setHeader('Content-Type', mime);
  res.setHeader('Content-Length', buffer.length);
  res.status(200).send(buffer);

}

If I have run Nextjs locally, this works fine. I can enter the API route directly into the browser and the image will render. However, if I run this in nextjs-lambda, I get an error. Specifically, the content length of the actual data returned is very much larger than the actual image size. I have an image of size 224214 and the content length reported by the server lambda is 403034 and the content is corrupted.

The issue is that the API Gateway (HTTP API) that sits in front of the lambda is not recognising the fact that the data being returned is binary and is mangling it (https://aws.amazon.com/blogs/compute/handling-binary-data-using-amazon-api-gateway-http-apis/). This library explicitly sets the response from the lambda to be non-binary: https://github.com/sladg/nextjs-lambda/blob/master/lib/standalone/server-handler.ts#L39

I don't see what the harm would be to remove that config flag: when we look at the way the serverless-http response from the Nextjs lambda is formatted, if the binary option is not specified then the response will not be treated as binary unless the underlying lambda returns a binary header or the Content-Type is set to be a binary type (by matching a type defined in process.env.BINARY_CONTENT_TYPES). Specifically, by not setting that flag we would hit this if statement.

Could we please consider removing that line?

This would allow for setting BINARY_CONTENT_TYPES=image/png,image/jpg,image/jpeg or similar and then such content would be returned to API Gateway in a way that APIG can handle it appropriately.

sladg commented 1 year ago

Hey! Yeah this scenario makes sense and I understand the value of allowing for binary data to be returned.

Looking at the serverless-http configuration, it might actually make sense to ditch the configuration and env variables and just set it to true?

That would basically allow any binary data to be returned from API routes without need to configuration, what you think?

It seems like binary is set to true by default anyways. https://github.com/dougmoscrop/serverless-http/blob/master/docs/ADVANCED.md#binary-mode

john-tipper commented 1 year ago

Thanks for looking at this!

Yes, on second thought I think that’s a better approach and makes it easier for end users to understand/reason about. Are you ok to amend or would you like me to amend my PR?

On 23 Nov 2022, at 12:09, Jan @.***> wrote:



Hey! Yeah this scenario makes sense and I understand the value of allowing for binary data to be returned.

Looking at the serverless-http configuration, it might actually make sense to ditch the configuration and env variables and just set it to true?

That would basically allow any binary data to be returned from API routes without need to configuration, what you think?

It seems like binary is set to true by default anyways. https://github.com/dougmoscrop/serverless-http/blob/master/docs/ADVANCED.md#binary-mode

— Reply to this email directly, view it on GitHubhttps://github.com/sladg/nextjs-lambda/issues/56#issuecomment-1324962046, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACKHSXSPMQDTAR5STUYEW2LWJYCQNANCNFSM6AAAAAASIMEE6Y. You are receiving this because you authored the thread.Message ID: @.***>

sladg commented 1 year ago

Feel free to edit your PR, I will be happy to approve. Alternatively, I will get to it over a weekend

john-tipper commented 1 year ago

Great, have updated the PR to always use binary mode.

On 23 Nov 2022, at 13:09, Jan @.***> wrote:



Feel free to edit your PR, I will be happy to approve. Alternatively, I will get to it over a weekend

— Reply to this email directly, view it on GitHubhttps://github.com/sladg/nextjs-lambda/issues/56#issuecomment-1325035309, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACKHSXWB2IKW3KIAXVSEBD3WJYJQNANCNFSM6AAAAAASIMEE6Y. You are receiving this because you authored the thread.Message ID: @.***>

sladg commented 1 year ago

Merged ✅