-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: throw and catch error when calls to cloudinary upload fails #72
Conversation
✅ Deploy Preview for netlify-plugin-cloudinary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -5,7 +5,7 @@ import { JSDOM } from 'jsdom' | |||
import { v2 as cloudinary } from 'cloudinary' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matiasfha can you check on line 330 of this file where the srcset is updated, can you verify an example with srcset will throw and catch the errors appropriately?
i'm not sure if we have a test for this and additionally may need to add it to the example being tested, but there's no try/catch around it like the other instnace of getCloudinaryUrl
return getCloudinaryUrl({ |
i created some conflicts in other work im doing, pushed a commit to fix the conflicts and catch up teh branch |
const cldAssetPath = `/${path.join(PUBLIC_ASSET_PATH, mediaPath)}`; | ||
const cldAssetUrl = `${host}${cldAssetPath}`; | ||
|
||
const { cloudinaryUrl: assetRedirectUrl } = await getCloudinaryUrl({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one thing im concerned about here is similar to a previous comment
if getCloudinaryUrl
throws an error here - wouldn't it throw an error for the entire try/catch
, thus preventing the plugin from continuing to upload other images in the array?
wouldn't it make more sense for the try to wrap the getCloudinaryUrl
call alone?
Updated |
thanks @matiasfha |
🎉 This PR is included in version 1.12.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Catch errors when using cloudinary uploader. Throw in the lib catch during build and log them .
Issue Ticket Number
Fixed #58
Type of change
Checklist