WalkingPizza / strapi-plugin-placeholder

Serve placeholders with your Strapi images
MIT License
29 stars 20 forks source link

Bug: Updating images leads to an 500 error #2

Closed Neihed closed 1 year ago

Neihed commented 2 years ago

error: Cannot read properties of undefined (reading 'startsWith')

If you update an image, the error above will be thrown because there is no mime property in the data object.

module.exports = ({ strapi }) => {
  /* Generate a placeholder when a new image is uploaded or updates */

  const generatePlaceholder = async (event) => {
    const { data } = event.params;
    if (!isValidMimeType(data.mime)) return; // data.mine is undefined.
    data.placeholder = await getService(strapi, 'placeholder').generate(data.url);
  };

  strapi.db.lifecycles.subscribe({
    models: ['plugin::upload.file'],
    beforeCreate: generatePlaceholder,
    beforeUpdate: generatePlaceholder,
  });
};
const isValidMimeType = (mimeType) => mimeType.startsWith('image/');
safirelauene commented 2 years ago

Having the same problem here when trying to update an image. Had to go back to version 4.1.5 to work around the issue for now

WalkingPizza commented 2 years ago

Apologies for the late reply, but I have been a bit short on time in the last couple of weeks! I have tried recreating the issue but wasn't able to. Could you please let me know how you are updating the images?

JakobLierman commented 2 years ago

You can easily reproduce this issue by changing the caption for an image in the Media Library. Currently using the following versions:

A quick workaround for now would be to patch the package. Here is the diff that solved the problem:

diff --git a/node_modules/strapi-plugin-placeholder/server/utils.js b/node_modules/strapi-plugin-placeholder/server/utils.js
index afee787..1d567ff 100644
--- a/node_modules/strapi-plugin-placeholder/server/utils.js
+++ b/node_modules/strapi-plugin-placeholder/server/utils.js
@@ -8,7 +8,7 @@ const pluginId = require('./plugin-id');
  * @returns {boolean} whether a placeholder can be generated for the given MIME type
  */

-const isValidMimeType = (mimeType) => mimeType.startsWith('image/');
+const isValidMimeType = (mimeType) => !!mimeType && mimeType.startsWith('image/');

 /**
  * Helper that retrieves one of the available services of this plugin from Strapi.
moneszarrugh commented 2 years ago

any update? this repo seems abandoned already

WalkingPizza commented 2 years ago

Apologies for the delay. I've been trying to move this plugin to the community org, but I guess I'll have to keep maintaining it myself.

I'll push a fix by tomorrow evening!

WalkingPizza commented 2 years ago

Currently troubleshooting with the Strapi team to understand why the mime type of the updated content is undefined. Going with @JakobLierman's solution is a temporary workaround, but it would cause issues in the long run. I will keep you guys updated as things evolve.

moneszarrugh commented 2 years ago

Currently troubleshooting with the Strapi team to understand why the mime type of the updated content is undefined. Going with @JakobLierman's solution is a temporary workaround, but it would cause issues in the long run. I will keep you guys updated as things evolve.

May I suggest this at least for now, until strapi team fix the bug.

https://www.npmjs.com/package/file-type

WalkingPizza commented 2 years ago

Should be fixed in the new 4.3.6 version!

Neihed commented 2 years ago

@WalkingPizza Thank you for your effort. Can we close this issue now? I have not testet it, yet

gregghawes commented 2 years ago

I'm still gets an error when updating an image.

TypeError: Cannot read properties of undefined (reading 'startsWith')

WalkingPizza commented 2 years ago

I'm still gets an error when updating an image.

TypeError: Cannot read properties of undefined (reading 'startsWith')

What version of the plugin do you have installed?

gregghawes commented 2 years ago

I'm still gets an error when updating an image. TypeError: Cannot read properties of undefined (reading 'startsWith')

What version of the plugin do you have installed?

"strapi-plugin-placeholder": "^4.3.6",
"@strapi/strapi": "4.3.8",
tefkah commented 2 years ago

I also get the same error, with

    "strapi-plugin-placeholder": "^4.3.6",
    "@strapi/strapi": "^4.4.3",
alexanderbnelson commented 1 year ago

Any plans to fix this any time soon? Hard to use this plugin with this issue.

julesrenaud commented 1 year ago

@WalkingPizza this is a very simple fix. You don't need the mime type when the file hasn't changed, so you don't get it in the data from the lifecycle hook. undefined is just fine. Don't try to update the placeholder data if the file hasn't changed. When the "replace media" function is used, the mime type is present in the lifecycle hook data and everything goes through fine.

The error, which is present in the current release, (file?.mime?.startsWith is not a function) emanates from trying to run startsWith on a boolean, which obviously throws an error. If there isn't an extension in the file name, mimeTypes.lookup returns false. You need to check if you can get the mime type from the file name before assuming you can.

Your code:

// strapi-plugin-placeholder/server/utils.js, line 12

const canGeneratePlaceholder = (file) => {
  if (!file.mime) file.mime = mimeTypes.lookup(file.name); // if there isn't an extension in the file name,  mimeTypes.lookup returns false
  return file.mime?.startsWith('image/'); // this will throw an error if file.mime is a boolean
};

This will fix the issue:

// strapi-plugin-placeholder/server/utils.js, line 12

const canGeneratePlaceholder = (file) => {
  if (!file.mime && mimeTypes.lookup(file.name))
    file.mime = mimeTypes.lookup(file.name);
  return file.mime?.startsWith('image/');
};

If you could patch this quickly it would be very much appreciated!

nikravi commented 1 year ago

Please review the PR https://github.com/WalkingPizza/strapi-plugin-placeholder/pull/12

yasssuz commented 1 year ago

Issue fixed.