NextJS 10.0.0 <Image /> component causes 404

Ali762 commented 4 years ago

Describe the bug

I realise NextJS 10 is only a couple of days old, but thought I'd just mention this to make sure it's captured. Using the new NextJS Component doesn't work.

Actual behavior

Using the Image component results in a 404 error from the server. It works in dev, but deploying to AWS doesn't.

Expected behavior

The image should return correctly.

Steps to reproduce

Screenshots/Code/Logs

Request URL: https://mywebsite.com/_next/image?url=https%3A%2F%2Fcdn.sanity.io%2Fimages%2F9h...etc etc Request Method: GET Status Code: 404

Response Headers: cache-control: public, max-age=0, s-maxage=2678400, must-revalidate content-encoding: gzip content-type: text/html date: Fri, 30 Oct 2020 01:42:59 GMT etag: W/"6805d78c611deceec9b5bd040ab84078" last-modified: Fri, 30 Oct 2020 01:37:01 GMT server: AmazonS3 status: 404 vary: Accept-Encoding via: 1.1 2b782f5f082f9e98adf8c50f24b6bb6d.cloudfront.net (CloudFront) x-amz-cf-id: XDhH5J8y7n052fhPQ6T9Rvh1yAptUwU9eh7sT1RQWbqBNNlyEGyuNw== x-amz-cf-pop: HAM50-C3 x-cache: Error from cloudfront

Versions

OS/Environment: @sls-next/serverless-component@1.17.0 "next": "^10.0.0",

dphang commented 4 years ago

Thanks for reporting - we'll prioritize the Next.js 10 features right after 1.18. Just want to address a couple small things before releasing 1.18.

nfantone commented 4 years ago

Came here for this. Experiencing the same, at the moment.

The Image component introduced in Next 10, generates URLs such as https://domain.cloudfront.net/_next/image?url=https%3A%2F%2domain.cloudfront.net%2Fclients%2Fclien-58ab57a5-3575-5f32-8eeb-e3b77d7b7757.svg&w=256&q=75 and they all currently return 404.

dphang commented 4 years ago

Investigating this now. It looks like there is a bit complex logic since it's on-demand optimization: https://github.com/vercel/next.js/blob/48acc479f3befb70de800392315831ed7defa4d8/packages/next/next-server/server/image-optimizer.ts

Will work on porting & optimizing this code for this component.

It does add some additional image transformation library, so we would probably want to also use Rollup.js to optimize this out to reduce handler size, if one doesn't use image components.

RichAWarren commented 4 years ago

Just wondering if there is a rough time span for releasing the next 10 image component support?

Bigs props to building/maintaining this package, it's awesome 👍

dphang commented 4 years ago

I am hoping to hopefully get it out around Thanksgiving (Nov 26).

The code is there from Next.js - just a matter of porting it and then optimizing it for a serverless environment as mentioned above using Rollup.js

confix commented 3 years ago

@dphang what's the ETA on that one? I'd really love to use it anytime soon. Is there anything I could help with?

dphang commented 3 years ago

@confix sorry for the delay, had been caught up in some personal stuff over the weekend. I'll try to get a PR out by this week.

dphang commented 3 years ago

Have created a draft PR above - still working on updating unit tests to maintain the code coverage, and cleaning up the code a little, though e2e tests have been added.

Feel free to pull that branch and try it out in the meanwhile and let me know if you find any bugs (note, you will need Node 12.x, set input imageOptimizer=true and also forward Accept headers on cloudfront.defaults input to properly return modern image formats e.g WEBP).

arelaxend commented 3 years ago

@dphang does this mean that it will affect cold start for the lambdas ? Just wondering what effects about increasing the bundle size so much 👍

Does this affect directly the performances of the lambdas that are already in place? Or it is non dependent (e.g. in a separate lambda).

Also, for previous RFC implementations there are charts to clarify the approach & solutions but here there is less documentations. It could help to add protocol charts for this feature too. What do you think ?

dphang commented 3 years ago

Good question! So from my experience adding code to the bundle has very little effect to cold start performance, it is mostly loading that code into the Node.js container that is slow. In the PR, none of the image optimization code is loaded on the critical path, they are dynamically loaded when an image request comes in. Though I have put it in default-lambda for now, but it can be easily refactored into its own Lambda. With +7 MB (due to the underlying sharp image processing binaries) you may see a few milliseconds added to cold start time to download the code (but not loaded until actually needed).

And adding 7 MB for the libraries required for image optimization may have other issues, namely that it may not fit into the 50 MB Lambda limit for some users. So maybe a new cache behavior (_next/image) and handler (image-lambda) should be created with a new Lambda instead? Image optimization is heavy so I'm probably leaning to its own lambda to keep other code paths unimpacted by cold starts / code size issues.

And using a separate Lambda adds a few other advantages:

Currently I copy a set of node_modules for sharp that's pre-built for Lambda, but it may interfere with the serverless-trace target if there are the same modules, since it also uses a node_modules directory, as opposed to bundling the requirements into each page. By using a separate Lambda, we can avoid this.
You can optionally increase the memory to the highest for images to also improve the CPU performance, since it is computation heavy (as recommended by the Sharp team: https://sharp.pixelplumbing.com/install#aws-lambda) while using a lower memory size for the API and default lambda). These images will still be cached, so you only pay for the initial request to a CloudFront edge node.
Easier to maintain and unit test.

@danielcondemarin let me know your thoughts.

danielcondemarin commented 3 years ago

Good question! So from my experience adding code to the bundle has very little effect to cold start performance, it is mostly loading that code into the Node.js container that is slow. In the PR, none of the image optimization code is loaded on the critical path, they are dynamically loaded when an image request comes in. Though I have put it in default-lambda for now, but it can be easily refactored into its own Lambda. With +7 MB (due to the underlying sharp image processing binaries) you may see a few milliseconds added to cold start time to download the code (but not loaded until actually needed).

And adding 7 MB for the libraries required for image optimization may have other issues, namely that it may not fit into the 50 MB Lambda limit for some users. So maybe a new cache behavior (_next/image) and handler (image-lambda) should be created with a new Lambda instead? Image optimization is heavy so I'm probably leaning to its own lambda to keep other code paths unimpacted by cold starts / code size issues.

And using a separate Lambda adds a few other advantages:

Currently I copy a set of node_modules for sharp that's pre-built for Lambda, but it may interfere with the serverless-trace target if there are the same modules, since it also uses a node_modules directory, as opposed to bundling the requirements into each page. By using a separate Lambda, we can avoid this.

You can optionally increase the memory to the highest for images to also improve the CPU performance, since it is computation heavy (as recommended by the Sharp team: https://sharp.pixelplumbing.com/install#aws-lambda) while using a lower memory size for the API and default lambda). These images will still be cached, so you only pay for the initial request to a CloudFront edge node.

Easier to maintain and unit test.

@danielcondemarin let me know your thoughts.

I'd also be inclined to having image optimisation in a separate Lambda. Whenever possible I think we should aim to keep the default handler lightweight as that's where most traffic is routed through. The only question I have is can we guarantee that Next will request the image from a URL that starts with _next/image/* ? I'm not super familiar with the new feature but seems like it doesn't support custom loaders - https://nextjs.org/docs/basic-features/image-optimization#loader.

dphang commented 3 years ago

The default Image component would use the loader on the same domain (the /_next/image paths) which is working in my draft PR.

It also supports a few other ones (Akamai etc) but not a custom loader i.e one of your own.

It also seems you can also use one of the built-in ones and just mimick its API as well as a way to support a custom loader. Proper custom loader support may also be coming soon. See: https://github.com/vercel/next.js/discussions/18450

danielcondemarin commented 3 years ago

The default Image component would use the loader on the same domain (the /_next/image paths) which is working in my draft PR.

It also supports a few other ones (Akamai etc) but not a custom loader i.e one of your own.

It also seems you can also use one of the built-in ones and just mimick its API as well as a way to support a custom loader. Proper custom loader support may also be coming soon. See: vercel/next.js#18450

Ah thanks for clarifying. In that case a custom cache behaviour for _next/image/* and its own Lambda sound like a nice approach! In future if custom loaders are available we could potentially add one for us but at this point I don't see much benefit in that if Next Server is going to keep _next/image in the URL.

dphang commented 3 years ago

@danielcondemarin thanks for confirming, I will refactor and I think this new image Lambda can be enabled by default as long as user is on Next.js 10 (which we can detect by the presence of image-manifest.json) since it's a separate path.

As for the custom loader, once supported I think we don't have to do anything, it would be the client-side component pointing to a different URL (Akamai, custom loader, etc.)

mark-a-bailey commented 3 years ago

I am not sure what version this was (is) resolved but as of 1.18.0 it is still causing a 404. It looks like the _next/image route isn't being setup in the CloudFront template. Can someone confirmed if it will be in 1.19?

ghost commented 3 years ago

Getting this issue as well using latest Next version & serverless

dphang commented 3 years ago

Are you trying the latest 1.19 alpha version? This has been there for some time now. Please confirm and open a new issue in case there are still problems - I may miss notifications especially on closed issues.

We will promote to 1.19 soon (not that alpha is unstable as it's well tested, but just giving some bake time to iron out some bugs). Thanks!

emulienfou commented 3 years ago

@dphang Getting the same 404 error with the version 1.19.0-alpha.32 and using a custom domain has the documentation requires to do. Like @mark-a-bailey says it looks like the _next/image route isn't being setup.

cushdan commented 3 years ago

@emulienfou I was able to get this working with the following serverless config changes (as found in the integration tests of the feature)

<your application>:
  component: "@sls-next/serverless-component@1.19.0-alpha.37"
  inputs:
    imageOptimizer: true
    bucketName: <your bucket>
    name:
      defaultLambda: <your default lambda>
      apiLambda: <your api lambda>
    cloudfront:
      distributionId: <your_distribution>
      defaults:
        forward:
          headers: [ Accept ] # Needed for image optimizer

emulienfou commented 3 years ago

~~@cushdan just tried with the same config likes component: "@sls-next/serverless-component@1.19.0-alpha.37" and headers: [ Accept ] this is not working on my side (with webp images). Did you change the next.config.js file also?~~

Never-mind I just needed to put the images inside the public folder. This configuration is working perfectly, thanks @cushdan

nfantone commented 3 years ago

FWIW, I kept my old config and just upgraded to latest -alpha and it did the trick. This is my whole serverless.yml.

myApplication:
  component: '@sls-next/serverless-component@1.19.0-alpha.37'
  name: recruitment-platform-ui
  org: myorg
  stage: integration
  app: my-application

redimongo commented 3 years ago

getting 404 error on plesk server can someone please assist and tell us why the following code is not working? it works fine on developer mode, fine on http://vercel.com servers but on plesk servers it has issues. and produces a 403 page

<Image alt={business.businessName} width="100px" height="100px" src="https://storage.googleapis.com/radiomedia-images/station_logos/DRN1Logo.png"/>

dphang commented 3 years ago

@redimongo I'm not sure about Plesk server - never used it but from a quick search it looks like traditional server hosting software? Are you sure you posted in the right repo? This is for Serverless-next.js which is deployed on AWS Lambda@Edge.

Also this issue is pretty old so would be better to open a new one if it's relevant to serverless-next.js. Thanks!

serverless-nextjs / serverless-next.js