strapi icon indicating copy to clipboard operation
strapi copied to clipboard

provider-upload-cloudinary missing a catch makes production app restart

Open gvocale opened this issue 2 years ago • 5 comments

May 13 01:24:34 PM  Error: Error uploading to cloudinary: Invalid image file    at /opt/render/project/src/node_modules/@strapi/provider-upload-cloudinary/lib/index.js:34:21    at /opt/render/project/src/node_modules/cloudinary/lib/utils/index.js:1243:14    at IncomingMessage.<anonymous> (/opt/render/project/src/node_modules/cloudinary/lib/uploader.js:507:9)    at IncomingMessage.emit (node:events:539:35)    at IncomingMessage.emit (node:domain:475:12)    at endReadableNT (node:internal/streams/readable:1345:12)    at processTicksAndRejections (node:internal/process/task_queues:83:21)
May 13 01:24:34 PM  This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason:
May 13 01:24:34 PM  { message: 'Invalid image file', name: 'Error', http_code: 400 }

Bug report

Required System information

  • Node.js version: 16
  • NPM version:
  • Strapi version: 4.1
  • Database: MariaDB
  • Operating system: Mac OS

Describe the bug

I am uploading files via node and @strapi/provider-upload-cloudinary. In some situation, an error thrown by cloudinary would make my production app crash and restart. The error says there is a missing catch for a promise, but I am already trying to catch errors this way:

      axios({
        method: "post",
        url: UPLOAD_API,
        data: formData,
        headers: formData.getHeaders(),
        maxBodyLength: Infinity,
        maxContentLength: Infinity,
      }).catch(function (error) {
        if (error.response) {
          Logger.log(
            error.response.data,
            error.response.status,
            error.response.headers
          );
        } else if (error.request) {
          Logger.log(error.request);
        } else {
          Logger.log("Error", error.message);
        }
        Logger.log(error.config, error.toJSON());
      });

Is there a way to catch the error in a way that won't trigger the server to restart?

gvocale avatar May 13 '22 17:05 gvocale

I have also tried to use onPart and filter in formidable to see if I can detect an invalid image file there, but without luck. This doesn't console log anything:

// middlewares.js
module.exports = [
  {
    name: "strapi::body",
    config: {
      multipart: true,
      onError: (err, ctx) => {
        console.log({ err, ctx });
      },
      formLimit: "256mb", // modify form body
      jsonLimit: "256mb", // modify JSON body
      textLimit: "256mb", // modify text body
      formidable: {
        onPart: function (part) {
          console.log("onPart", { part });
          // keep only images
        },
        filter(_part) {
          console.log("part", { part });
          return true;
        },
        // Cloudinary max upload size is 41943040.
        maxFileSize: 41943040, // multipart data, modify here limit of uploaded file size
      },
    },
  },

gvocale avatar May 13 '22 21:05 gvocale

related to but not duplicate of: https://github.com/strapi/strapi/issues/13132

derrickmehaffy avatar May 18 '22 23:05 derrickmehaffy

Hi :) How can I reproduce the bug?

petersg83 avatar May 19 '22 07:05 petersg83

Hi :) How can I reproduce the bug?

You would basically need to upload a file that cloudinary rejects like a PDF but you can use #13132 as an example as this would be basically the same thing. If we fix one of them, we fix the other.

derrickmehaffy avatar May 25 '22 18:05 derrickmehaffy

My patch of @strapi/provider-upload-cloudinary to fix that if it can be helpful to anyone:

diff --git a/node_modules/@strapi/provider-upload-cloudinary/lib/index.js b/node_modules/@strapi/provider-upload-cloudinary/lib/index.js
index 1cc488a..02210d4 100644
--- a/node_modules/@strapi/provider-upload-cloudinary/lib/index.js
+++ b/node_modules/@strapi/provider-upload-cloudinary/lib/index.js
@@ -14,7 +14,7 @@ module.exports = {
     cloudinary.config(config);
 
     const upload = (file, customConfig = {}) =>
-      new Promise(resolve => {
+      new Promise((resolve, reject) => {
         const config = {
           resource_type: 'auto',
           public_id: file.hash,
@@ -29,9 +29,9 @@ module.exports = {
           (err, image) => {
             if (err) {
               if (err.message.includes('File size too large')) {
-                throw new PayloadTooLargeError();
+                return reject(new PayloadTooLargeError());
               }
-              throw new Error(`Error uploading to cloudinary: ${err.message}`);
+              return reject(new Error(`Error uploading to cloudinary: ${err.message}`));
             }
 
             if (image.resource_type === 'video') {

VictorienTardif avatar Jul 13 '22 13:07 VictorienTardif

Closing this issue as it was fixed in https://github.com/strapi/strapi/pull/13992 Thank you @VictorienTardif for the provided fix.

Marc-Roig avatar Aug 12 '22 08:08 Marc-Roig