Assets: Handle transformation timeout errors

[joselcvarela]

Motivation

When requesting a asset transformation and if the transformation takes longer than ASSETS_TRANSFORM_TIMEOUT the response for the request will be 500 Internal Server Error.

Also, if the asset transformation fails, the transformation file is not deleted from storage ending with a 0 byte file.
This means if the transformation failed, for example, due to timeout and then if we increase the timeout and request the same transformation, it will always return the 0 byte transformation file.

Solution

Since we are using sharp module to handle assets transformations and since this is using libvips which is a C/C++ library to manipulate images, the option ASSETS_TRANSFORM_TIMEOUT is passed to this libvips library and when the timeout is reached, this library sends an error to sharp and at Directus level we just receive an Error object with message property equal to timeout: NN% complete\nVipsImage: killed for image YYYYY, where NN is the percentage at it was aborted and YYYYY is the temporary file name that libvips gave to the current file.

With that being said, the solution was just simply a try/catch block around the transformation pipe and respond with a 408 Request Timeout (which seemed the best suited status code).
Since 408 requests are retried, this can make the next request successful if server has more free resources for the new request.

Plus, we are deleting the transformation file in case the transformation went wrong and a 0 byte file was saved.
This way, server will try to do another transformation instead of returning an empty file.

[changeset-bot]

🦋 Changeset detected

Latest commit: e8f0ed4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@directus/api	Patch
directus	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[joselcvarela]

In order to test this I have used these configurations, so I could test the transformation timeout:

docker-compose.yml

  directus-debugger:
    build:
      context: .
      dockerfile: Dockerfile-test

    ports:
      - 8055:8055
      - 9229:9229
    volumes:
      - ".:/app"
      - "./docker_node_modules:/app/node_modules"
    deploy:
      resources:
        limits:
          cpus: '0.4'
          memory: 512M
        reservations:
          cpus: '0.3'
          memory: 256M
    working_dir: /app/api
    command: node --inspect=0.0.0.0 dist/cli/run.js start

Dockerfile-test

FROM node:18-alpine

RUN apk add python3 make g++ vips-dev

RUN npm i -g pnpm@8.6.10

[azrikahar]

Can confirm the try/catch works for me!

I'm just curious on others' thoughts on whether returning HTTP status code 408 is correct here?

I'm interpreting it as a status code to return when the user's request took too long to completely be sent to the server, especially since 4xx codes are generally for client side errors.

However in this transformation timeout case, it's technically the server, or rather libvips, that is timing out since it was not able to process the file-to-be-transformed in time.

That being said, I could be misinterpreting the status code entirely!

[paescuj]

Can confirm the try/catch works for me!

I'm just curious on others' thoughts on whether returning HTTP status code 408 is correct here?

I'm interpreting it as a status code to return when the user's request took too long to completely be sent to the server, especially since 4xx codes are generally for client side errors.

However in this transformation timeout case, it's technically the server, or rather libvips, that is timing out since it was not able to process the file-to-be-transformed in time.

That being said, I could be misinterpreting the status code entirely!

Absolutely! If we do want to return another status code here, I guess it should be 503 Service Unavailable.

[br41nslug]

I can confirm it works 😄

However i do have my doubts about this 408 error. Browser do try to repeat the request which can be both a blessing and a curse 😬

• If the timeout is a one-off then you want it to retry because the chance is high it will succeed the next attempt
• If the timeout is due to constant highload/not enough resources then retrying the request will only make the problem worse for the server

I tend to agree with @azrikahar that 4xx errors are client errors and what we're throwing here is a server error.

[joselcvarela]

After reading the comments and check the reference again, you guys are right.
I've dropped the RequestTimeoutError in favor of ServiceUnavailableError