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

NextJS 10.0.0 <Image /> component causes 404 #725

Closed
Ali762 opened this issue Oct 30, 2020 · 23 comments · Fixed by #829
Closed

NextJS 10.0.0 <Image /> component causes 404 #725

Ali762 opened this issue Oct 30, 2020 · 23 comments · Fixed by #829
Assignees
Labels
enhancement New feature or request release-1.19

Comments

@Ali762
Copy link

Ali762 commented Oct 30, 2020

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 dphang added the enhancement New feature or request label Oct 30, 2020
@dphang
Copy link
Collaborator

dphang commented Oct 30, 2020

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
Copy link

nfantone commented Nov 5, 2020

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 dphang self-assigned this Nov 5, 2020
@dphang
Copy link
Collaborator

dphang commented Nov 11, 2020

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
Copy link

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
Copy link
Collaborator

dphang commented Nov 11, 2020

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
Copy link
Contributor

confix commented Nov 30, 2020

@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
Copy link
Collaborator

dphang commented Nov 30, 2020

@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
Copy link
Collaborator

dphang commented Dec 2, 2020

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
Copy link

arelaxend commented Dec 2, 2020

@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
Copy link
Collaborator

dphang commented Dec 2, 2020

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
Copy link
Contributor

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
Copy link
Collaborator

dphang commented Dec 2, 2020

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

@danielcondemarin
Copy link
Contributor

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
Copy link
Collaborator

dphang commented Dec 2, 2020

@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
Copy link

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
Copy link

ghost commented Jan 29, 2021

Getting this issue as well using latest Next version & serverless

@dphang
Copy link
Collaborator

dphang commented Jan 29, 2021

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
Copy link

@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
Copy link
Contributor

cushdan commented Mar 17, 2021

@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
Copy link

emulienfou commented Mar 17, 2021

@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
Copy link

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

rwalle61 added a commit to bingooal/site that referenced this issue Apr 1, 2021
Next Serverless component has a bug where urls of optimised images
are not found when deployed. The component is fixed in the alpha
version,
see serverless-nextjs/serverless-next.js#725 (comment)
@redimongo
Copy link

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
Copy link
Collaborator

dphang commented Aug 6, 2021

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request release-1.19
Projects
None yet
Development

Successfully merging a pull request may close this issue.