Skip to content

Commit

Permalink
fix: Fixes uncaught errors in concurrency loop (#95)
Browse files Browse the repository at this point in the history
# Description

Concurrency was added for parallel uploads which introduced a bug where
the try/catch wrapping this didn't properly catch the errors being
thrown from within the async functions.

This moves the try/catch inside of each upload process

## Issue Ticket Number

<!-- Specifiy which issue this fixes by referencing the issue number
(`#11`) or issue URL. -->
<!-- Example: Fixes
#1 -->

Fixes #94 

## Type of change

<!-- Please select all options that are applicable. -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update


# Checklist

<!-- These must all be followed and checked. -->

- [ ] I have followed the contributing guidelines of this project as
mentioned in [CONTRIBUTING.md](/CONTRIBUTING.md)
- [ ] I have created an
[issue](https://github.com/colbyfayock/netlify-plugin-cloudinary/issues)
ticket for this PR
- [ ] I have checked to ensure there aren't other open [Pull
Requests](https://github.com/colbyfayock/netlify-plugin-cloudinary/pulls)
for the same update/change?
- [ ] I have performed a self-review of my own code
- [ ] I have run tests locally to ensure they all pass
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes needed to the documentation
  • Loading branch information
colbyfayock committed Nov 10, 2023
1 parent df8c8c9 commit 2c6a33b
Showing 1 changed file with 12 additions and 12 deletions.
24 changes: 12 additions & 12 deletions netlify-plugin-cloudinary/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,12 @@ export async function onBuild({
);
}

try {
const limitUploadFiles = pLimit(uploadConcurrency);
const uploadsQueue = imagesFiles.map(image => {
const publishPath = image.replace(PUBLISH_DIR, '');
return limitUploadFiles(() => {
async function uploadFile() {
const limitUploadFiles = pLimit(uploadConcurrency);
const uploadsQueue = imagesFiles.map((image, i) => {
const publishPath = image.replace(PUBLISH_DIR, '');
return limitUploadFiles(() => {
async function uploadFile() {
try {
const cloudinary = await getCloudinaryUrl({
deliveryType,
folder,
Expand All @@ -224,15 +224,15 @@ export async function onBuild({
publishPath,
...cloudinary,
};
} catch(e) {
globalErrors.push(e);
}
return uploadFile();
})
}
return uploadFile();
})
})

_cloudinaryAssets.images = await Promise.all(uploadsQueue);
} catch (e) {
globalErrors.push(e)
}
_cloudinaryAssets.images = await Promise.all(uploadsQueue);

// If the delivery type is set to upload, we need to be able to map individual assets based on their public ID,
// which would require a dynamic middle solution, but that adds more hops than we want, so add a new redirect
Expand Down

0 comments on commit 2c6a33b

Please sign in to comment.