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

Bug: productBulkCreate fails with unresized Saleor image URLs #14508

Open
1 task
krzysztofwolski opened this issue Oct 27, 2023 · 2 comments
Open
1 task

Bug: productBulkCreate fails with unresized Saleor image URLs #14508

krzysztofwolski opened this issue Oct 27, 2023 · 2 comments
Assignees
Labels

Comments

@krzysztofwolski
Copy link
Member

krzysztofwolski commented Oct 27, 2023

What are you trying to achieve?

Create products using productBulkCreate and media already uploaded to Saleor.

Important context:
There are two "types" of image URLs returned by Saleor, based on if it was requested previously. When resized image URL was never requested before, it follows format:
https://XXX.saleor.cloud/thumbnail/UHJvZHVjdE1lhOjI0NA==/4096/ (no file name and extension)

After the URL was visited (requested) the image is resized and during the next query to the API url follows the format:
https://xxx.saleor.cloud/media/thumbnails/products/saleordemoproduct_fd_juice_02_thumbnail_4096.png (file name and extension included).

The bug occurs when the first type of URL is used (no file name or extension)

Steps to reproduce the problem

  1. Mutation:
mutation BulkCreate{
  productBulkCreate(errorPolicy: REJECT_FAILED_ROWS, products:[
    {name: "usingMedia", attributes: [], productType: "UHJvZHVjdFR5cGU6Njk4", media: [{alt: "test", mediaUrl:"https://v314.staging.saleor.cloud/thumbnail/UHJvZHVjdE1lZGlhOjI0NQ==/4096/"}]},
  ]){
    results{
      product{
        id
        name
        slug
      }
    }errors{
      path
      message
      code
    }
  }
}

Response:

{
  "errors": [
    {
      "message": "Internal Server Error",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "productBulkCreate"
      ],
      "extensions": {
        "exception": {
          "code": "TypeError"
        }
      }
    }
  ],
  "data": {
    "productBulkCreate": null
  },
  "extensions": {
    "cost": {
      "requestedQueryCost": 0,
      "maximumAvailable": 50000
    }
  }
}

What did you expect to happen?

Product with media be created

Logs

No response

Environment

Saleor version: 3.14.30
OS and version: …

Tasks

  1. enhancement
    NyanKiyoshi
@maarcingebala
Copy link
Member

maarcingebala commented Oct 30, 2023

When running the mutation for a single product, it raises a ValidationError:

mutation productMediaCreate {
  productMediaCreate(input: {product: "UHJvZHVjdDo3NDA=", mediaUrl: "https://v314.staging.saleor.cloud/thumbnail/UHJvZHVjdE1lZGlhOjI0NQ==/4096/"}) {
    errors {
      field
      message
    }
    product {
      id
    }
  }
}

Result:

"data": {
    "productMediaCreate": {
      "errors": [
        {
          "field": "mediaUrl",
          "message": "Unsupported media provider or incorrect URL."
        }
      ],
      "product": null
    }
  },

There are two problems here:

  • productBulkCreate is raising the same error as productMediaCreate in the get_oembed_data function, but the error is not wrapped with ValidationError, thus it fails with "Internal Server Error" (example log from Sentry). We should fix the mutation to catch errors from get_oembed_data and transform them to handled ValidationError.
  • the actual problem here is that our is_image_url function doesn't recognize such URLs https://v314.staging.saleor.cloud/thumbnail/UHJvZHVjdE1lZGlhOjI0NQ==/4096/ as image URL. We could extend this function with a check that tries to match the regex of our thumbnail's view, and if yes treat is as an image.

@maarcingebala
Copy link
Member

After internal discussion, here is the proposed solution:

  • drop the is_image_url function - we shouldn't assume that URL represents an image just by the looks of the URL; instead, we should download the file and check its mime type.
  • downloading the file and checking the mime type is already done in the validate_image_url, which should be used instead of is_image_url.
  • fix the handle_thumbnail view to always set the Content-Type header in the redirect responses (we should be able to get mime-type from the image file)
  • in both mutations productMediaCreate and productBulkCreate, we should allow redirects when fetching the actual image data image_data = HTTPClient.send_request(...)
  • Extra fix: the handle_thumbnail uses 302 redirect code, but it should be a permanent redirect code, this can be fixed by using HttpResponsePermanentRedirect redirect class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants