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

External source images are not responded from the repeated requests #12

Closed
b6pzeusbc54tvhw5jgpyw8pwz2x6gs opened this issue Mar 1, 2021 · 3 comments · Fixed by #13
Closed
Labels
bug Something isn't working

Comments

@b6pzeusbc54tvhw5jgpyw8pwz2x6gs
Copy link
Contributor

b6pzeusbc54tvhw5jgpyw8pwz2x6gs commented Mar 1, 2021

Test environment

In examples/with-next-js, I modify main.tf file like below:

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "~> 3.0"
    }
  }
}

provider "aws" {
  region = "us-east-1"
}

module "next_image_optimizer" {
  source = "dealmore/next-js-image-optimization/aws"

  next_image_domains = ["assets.vercel.com","cdn11.bigcommerce.com"]
  next_image_image_sizes  = [12, 24, 48]
  next_image_device_sizes = [760, 960, 1024]
}

output "domain" {
  value = module.next_image_optimizer.cloudfront_domain_name
}

then terraform apply done successfully.

External source image: https://cdn11.bigcommerce.com/s-qfzerv205w/images/stencil/original/products/116/512/Men-Jacket-Front-Black__15466.1603283963.png

Test url: https://d1phddo1jfr778.cloudfront.net/_next/image/?url=https%3A%2F%2Fcdn11.bigcommerce.com%2Fs-qfzerv205w%2Fimages%2Fstencil%2Foriginal%2Fproducts%2F116%2F512%2FMen-Jacket-Front-Black__15466.1603283963.png&w=760&q=94

(d1phddo1jfr778.cloudfront.net will be destroyed in a few days)

Expected behavior

All repeated requests could get download images.

Current behavior

Requests with ETag header (with browser cache) works well.
But cache clear environments(without ETag) can not get images.
I use chrome browser Incognito mode for cache clear.

2021-03-02 02 07 54

Solution

I reviewed code in this repo and nextjs image-optimizer then finally found a suspicious point.

I Think in the ./lib/handler.ts handler function's return should wait steam write. (next image-optimizer use readStream.pipe(res))

@ofhouse Could I make PR?

@b6pzeusbc54tvhw5jgpyw8pwz2x6gs b6pzeusbc54tvhw5jgpyw8pwz2x6gs changed the title External source images are not responded from the 2nd request External source images are not responded from the repeated requests Mar 1, 2021
@ofhouse
Copy link
Member

ofhouse commented Mar 1, 2021

Strange, currently I don't get why this works in the e2e:tests (they aren't using etags either).
But yeah seems like something is clearly broken there.

Would welcome a PR for this since I don't have much time the next 2 weeks to fix it by myself 👍

@b6pzeusbc54tvhw5jgpyw8pwz2x6gs
Copy link
Contributor Author

b6pzeusbc54tvhw5jgpyw8pwz2x6gs commented Mar 1, 2021

@ofhouse Wrong response' status is 200 but binary data is empty. The expected e2e test that should catch this problem is External image: Accept */*: %s and External image: Accept image/webp: %s ?

@ofhouse
Copy link
Member

ofhouse commented Mar 1, 2021

It would probably make sense to create a new test block for this case since it doesn't make sense to run through all fixtures for this.

ofhouse added a commit that referenced this issue Mar 15, 2021
…ng-response-when-repeated-requests-for-external-source

fix wrong response when repeated requests for external source (#12)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants