Skip to content

Commit c90c46f

Browse files
matiasfhamatiasfha
authored
feat: throw and catch error when calls to cloudinary upload fails (#72)
# Description <!-- Include a summary of the change made and also list the dependencies that are required if any --> Catch errors when using cloudinary uploader. Throw in the lib catch during build and log them . ## Issue Ticket Number Fixed #58 ![CleanShot 2023-09-21 at 11 40 00](https://github.com/cloudinary-community/netlify-plugin-cloudinary/assets/282006/07980b15-6e67-4cce-a54a-893169c0daab) ## Type of change <!-- Please select all options that are applicable. --> - [ ] Bug fix (non-breaking change which fixes an issue) - [X] 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. --> - [X] I have followed the contributing guidelines of this project as mentioned in [CONTRIBUTING.md](/CONTRIBUTING.md) - [X] I have created an [issue](https://github.com/colbyfayock/netlify-plugin-cloudinary/issues) ticket for this PR - [X] 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? - [X] I have performed a self-review of my own code - [X] I have run tests locally to ensure they all pass - [X] I have commented my code, particularly in hard-to-understand areas - [X] I have made corresponding changes needed to the documentation
1 parent f39cf30 commit c90c46f

File tree

4 files changed

+136
-89
lines changed

4 files changed

+136
-89
lines changed

‎netlify-plugin-cloudinary/src/data/errors.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
export const ERROR_API_CREDENTIALS_REQUIRED =
2-
'Both your Cloudinary API Key and API Secret are required when using a Delivery Type of Upload. Please confirm the environment variables CLOUDINARY_API_KEY and CLOUDINARY_API_SECRET are configured.';
3-
export const ERROR_CLOUD_NAME_REQUIRED =
4-
'A Cloudinary Cloud Name is required. Please set cloudName input or use the environment variable CLOUDINARY_CLOUD_NAME';
1+
export const ERROR_ASSET_UPLOAD = 'Error uploading asset'
2+
export const ERROR_API_CREDENTIALS_REQUIRED = 'Both your Cloudinary API Key and API Secret are required when using a Delivery Type of Upload. Please confirm the environment variables CLOUDINARY_API_KEY and CLOUDINARY_API_SECRET are configured.';
3+
export const ERROR_CLOUD_NAME_REQUIRED = 'A Cloudinary Cloud Name is required. Please set cloudName input or use the environment variable CLOUDINARY_CLOUD_NAME';
54
export const ERROR_INVALID_IMAGES_PATH = 'Invalid asset path. Please make sure your imagesPath is defined.';
6-
export const ERROR_NETLIFY_HOST_CLI_SUPPORT =
7-
'Note: The Netlify CLI does not currently support the ability to determine the host locally, try deploying on Netlify.';
8-
export const ERROR_NETLIFY_HOST_UNKNOWN =
9-
'Cannot determine Netlify host, can not proceed with plugin.';
10-
export const ERROR_SITE_NAME_REQUIRED = 'Cannot determine the site name, can not proceed with plugin';
5+
export const ERROR_NETLIFY_HOST_CLI_SUPPORT = 'Note: The Netlify CLI does not currently support the ability to determine the host locally, try deploying on Netlify.';
6+
export const ERROR_NETLIFY_HOST_UNKNOWN = 'Cannot determine Netlify host, can not proceed with plugin.';
7+
export const ERROR_SITE_NAME_REQUIRED = 'Cannot determine the site name, can not proceed with plugin';
8+
export const ERROR_UPLOAD_PRESET = 'To use a delivery type of "upload", please use an uploadPreset for unsigned requests or an API Key and Secret for signed requests'
9+
export const ERROR_INVALID_SRCSET = 'Invalid srcset path. Please make sure the srcset is defined.'

‎netlify-plugin-cloudinary/src/index.ts

Lines changed: 61 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ const CLOUDINARY_ASSET_DIRECTORIES = [
107107
*/
108108

109109
const _cloudinaryAssets = { images: {} } as Assets;
110+
const globalErrors = [];
110111

111112
export async function onBuild({
112113
netlifyConfig,
@@ -118,7 +119,7 @@ export async function onBuild({
118119

119120
let host = process.env.URL;
120121

121-
if ( process.env.CONTEXT === 'branch-deploy' || process.env.CONTEXT === 'deploy-preview' ) {
122+
if (process.env.CONTEXT === 'branch-deploy' || process.env.CONTEXT === 'deploy-preview') {
122123
host = process.env.DEPLOY_PRIME_URL || ''
123124
}
124125

@@ -139,6 +140,7 @@ export async function onBuild({
139140
} = inputs;
140141

141142
if (!folder) {
143+
console.error(`[Cloudinary] ${ERROR_SITE_NAME_REQUIRED}`);
142144
utils.build.failPlugin(ERROR_SITE_NAME_REQUIRED);
143145
return;
144146
}
@@ -182,6 +184,7 @@ export async function onBuild({
182184
// asset details and to grab a Cloudinary URL to use later
183185

184186
if (typeof imagesPath === 'undefined') {
187+
console.error(`[Cloudinary] ${ERROR_INVALID_IMAGES_PATH}`)
185188
throw new Error(ERROR_INVALID_IMAGES_PATH);
186189
}
187190

@@ -219,13 +222,7 @@ export async function onBuild({
219222
}),
220223
);
221224
} catch (e) {
222-
console.error('Error', e);
223-
if (e instanceof Error) {
224-
utils.build.failBuild(e.message);
225-
} else {
226-
utils.build.failBuild(e as string);
227-
}
228-
return;
225+
globalErrors.push(e)
229226
}
230227

231228
// If the delivery type is set to upload, we need to be able to map individual assets based on their public ID,
@@ -236,15 +233,18 @@ export async function onBuild({
236233
await Promise.all(
237234
Object.keys(_cloudinaryAssets).flatMap(mediaType => {
238235
// @ts-expect-error what are the expected mediaTypes that will be stored in _cloudinaryAssets
239-
return _cloudinaryAssets[mediaType].map(async asset => {
240-
const { publishPath, cloudinaryUrl } = asset;
241-
netlifyConfig.redirects.unshift({
242-
from: `${publishPath}*`,
243-
to: cloudinaryUrl,
244-
status: 302,
245-
force: true,
236+
if (Object.hasOwn(_cloudinaryAssets[mediaType], 'map')) {
237+
// @ts-expect-error what are the expected mediaTypes that will be stored in _cloudinaryAssets
238+
return _cloudinaryAssets[mediaType].map(async asset => {
239+
const { publishPath, cloudinaryUrl } = asset;
240+
netlifyConfig.redirects.unshift({
241+
from: `${publishPath}*`,
242+
to: cloudinaryUrl,
243+
status: 302,
244+
force: true,
245+
});
246246
});
247-
});
247+
}
248248
}),
249249
);
250250
}
@@ -261,8 +261,7 @@ export async function onBuild({
261261

262262
// Unsure how to type the above so that Inputs['privateCdn'] doesnt mess up types here
263263

264-
if (!Array.isArray(mediaPaths) && typeof mediaPaths !== 'string')
265-
return;
264+
if (!Array.isArray(mediaPaths) && typeof mediaPaths !== 'string') return;
266265

267266
if (!Array.isArray(mediaPaths)) {
268267
mediaPaths = [mediaPaths];
@@ -271,35 +270,36 @@ export async function onBuild({
271270
mediaPaths.forEach(async mediaPath => {
272271
const cldAssetPath = `/${path.join(PUBLIC_ASSET_PATH, mediaPath)}`;
273272
const cldAssetUrl = `${host}${cldAssetPath}`;
274-
275-
const { cloudinaryUrl: assetRedirectUrl } = await getCloudinaryUrl({
276-
deliveryType: 'fetch',
277-
folder,
278-
path: `${cldAssetUrl}/:splat`,
279-
uploadPreset,
280-
transformations
281-
});
282-
283-
netlifyConfig.redirects.unshift({
284-
from: `${cldAssetPath}/*`,
285-
to: `${mediaPath}/:splat`,
286-
status: 200,
287-
force: true,
288-
});
289-
290-
netlifyConfig.redirects.unshift({
291-
from: `${mediaPath}/*`,
292-
to: assetRedirectUrl,
293-
status: 302,
294-
force: true,
295-
});
296-
});
297-
},
298-
),
299-
);
273+
try {
274+
const { cloudinaryUrl: assetRedirectUrl } = await getCloudinaryUrl({
275+
deliveryType: 'fetch',
276+
folder,
277+
path: `${cldAssetUrl}/:splat`,
278+
uploadPreset,
279+
});
280+
281+
netlifyConfig.redirects.unshift({
282+
from: `${cldAssetPath}/*`,
283+
to: `${mediaPath}/:splat`,
284+
status: 200,
285+
force: true,
286+
});
287+
288+
netlifyConfig.redirects.unshift({
289+
from: `${mediaPath}/*`,
290+
to: assetRedirectUrl,
291+
status: 302,
292+
force: true,
293+
});
294+
} catch (error) {
295+
globalErrors.push(error)
296+
}
297+
})
298+
})
299+
)
300300
}
301301

302-
console.log('[Cloudinary] Done.');
302+
303303
}
304304

305305
// Post build looks through all of the output HTML and rewrites any src attributes to use a cloudinary URL
@@ -314,7 +314,7 @@ export async function onPostBuild({
314314

315315
let host = process.env.URL;
316316

317-
if ( process.env.CONTEXT === 'branch-deploy' || process.env.CONTEXT === 'deploy-preview' ) {
317+
if (process.env.CONTEXT === 'branch-deploy' || process.env.CONTEXT === 'deploy-preview') {
318318
host = process.env.DEPLOY_PRIME_URL || ''
319319
}
320320

@@ -331,6 +331,7 @@ export async function onPostBuild({
331331
} = inputs;
332332

333333
if (!folder) {
334+
console.error(`[Cloudinary] ${ERROR_SITE_NAME_REQUIRED}`);
334335
utils.build.failPlugin(ERROR_SITE_NAME_REQUIRED);
335336
return;
336337
}
@@ -392,11 +393,19 @@ export async function onPostBuild({
392393
);
393394

394395
const errors = results.filter(({ errors }) => errors.length > 0);
396+
// Collect the errors in the global scope to be used in the summary onEnd
397+
globalErrors.push(...errors)
395398

396-
if (errors.length > 0) {
397-
console.log(`[Cloudinary] Done with ${errors.length} errors...`);
398-
console.log(JSON.stringify(errors, null, 2));
399-
} else {
400-
console.log('[Cloudinary] Done.');
401-
}
399+
}
400+
401+
402+
export function onEnd({ utils }: { utils: Utils }) {
403+
const summary = globalErrors.length > 0 ? `Cloudinary build plugin completed with ${globalErrors.length} errors` : "Cloudinary build plugin completed successfully"
404+
const text = globalErrors.length > 0 ? `The build process found ${globalErrors.length} errors. Check build logs for more information` : "No errors found during build"
405+
utils.status.show({
406+
title: "[Cloudinary] Done.",
407+
// Required.
408+
summary,
409+
text
410+
});
402411
}

‎netlify-plugin-cloudinary/src/lib/cloudinary.ts

Lines changed: 49 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { JSDOM } from 'jsdom'
55
import { v2 as cloudinary, ConfigOptions, TransformationOptions } from 'cloudinary'
66

77
import { isRemoteUrl, determineRemoteUrl } from './util'
8-
import { ERROR_CLOUD_NAME_REQUIRED } from '../data/errors'
8+
import { ERROR_API_CREDENTIALS_REQUIRED, ERROR_ASSET_UPLOAD, ERROR_CLOUD_NAME_REQUIRED, ERROR_UPLOAD_PRESET } from '../data/errors'
99

1010
import { Inputs } from '../types/integration';
1111

@@ -95,7 +95,7 @@ export function configureCloudinary(config: CloudinaryConfig) {
9595
secure: true
9696
}
9797

98-
if ( config.cname ) {
98+
if (config.cname) {
9999
cloudinaryConfig.secure_distribution = config.cname;
100100
// When configuring a cname, we need to additionally set private CDN
101101
// to be true in order to work properly, which may not be obvious
@@ -155,13 +155,14 @@ export async function getCloudinaryUrl(options: CloudinaryOptions) {
155155
const canSignUpload = apiKey && apiSecret
156156

157157
if (!cloudName) {
158-
throw new Error(ERROR_CLOUD_NAME_REQUIRED)
158+
throw new Error(`[Cloudinary] ${ERROR_CLOUD_NAME_REQUIRED}`)
159159
}
160160

161161
if (deliveryType === 'upload' && !canSignUpload && !uploadPreset) {
162-
throw new Error(
163-
`To use deliveryType ${deliveryType}, please use an uploadPreset for unsigned requests or an API Key and Secret for signed requests.`,
164-
)
162+
if (!uploadPreset) {
163+
throw new Error(`[Cloudinary] ${ERROR_UPLOAD_PRESET}`)
164+
}
165+
throw new Error(`[Cloudinary] ${ERROR_API_CREDENTIALS_REQUIRED}`)
165166
}
166167

167168
let fileLocation
@@ -206,20 +207,31 @@ export async function getCloudinaryUrl(options: CloudinaryOptions) {
206207
if (canSignUpload) {
207208
// We need an API Key and Secret to use signed uploading
208209

209-
results = await cloudinary.uploader.upload(fullPath, {
210-
...uploadOptions,
211-
})
210+
try {
211+
results = await cloudinary.uploader.upload(fullPath, {
212+
...uploadOptions,
213+
})
214+
} catch (error) {
215+
console.error(`[Cloudinary] ${ERROR_ASSET_UPLOAD}`)
216+
console.error(`[Cloudinary] \tpath: ${fullPath}`)
217+
throw Error(ERROR_ASSET_UPLOAD)
218+
}
212219
} else {
213220
// If we want to avoid signing our uploads, we don't need our API Key and Secret,
214221
// however, we need to provide an uploadPreset
215-
216-
results = await cloudinary.uploader.unsigned_upload(
217-
fullPath,
218-
uploadPreset,
219-
{
220-
...uploadOptions,
221-
},
222-
)
222+
try {
223+
results = await cloudinary.uploader.unsigned_upload(
224+
fullPath,
225+
uploadPreset,
226+
{
227+
...uploadOptions,
228+
},
229+
)
230+
} catch (error) {
231+
console.error(`[Cloudinary] ${ERROR_ASSET_UPLOAD}`)
232+
console.error(`[Cloudinary] path: ${fullPath}`)
233+
throw Error(ERROR_ASSET_UPLOAD)
234+
}
223235
}
224236

225237
// Finally use the stored public ID to grab the image URL
@@ -340,14 +352,25 @@ export async function updateHtmlImagesToCloudinary(html: string, options: Update
340352
if (exists && deliveryType === 'upload') {
341353
return exists.cloudinaryUrl
342354
}
343-
return getCloudinaryUrl({
344-
deliveryType,
345-
folder,
346-
path: url[0],
347-
localDir,
348-
uploadPreset,
349-
remoteHost,
350-
})
355+
try {
356+
357+
return getCloudinaryUrl({
358+
deliveryType,
359+
folder,
360+
path: url[0],
361+
localDir,
362+
uploadPreset,
363+
remoteHost,
364+
})
365+
} catch (e) {
366+
if (e instanceof Error) {
367+
errors.push({
368+
imgSrc,
369+
message: e.message
370+
})
371+
}
372+
373+
}
351374
})
352375

353376
const srcsetUrlsCloudinary = await Promise.all(srcsetUrlsPromises)
@@ -397,4 +420,4 @@ export function getTransformationsFromInputs(inputs: Inputs) {
397420
})
398421
}
399422
return transformations;
400-
}
423+
}

‎netlify-plugin-cloudinary/tests/lib/cloudinary.test.js

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
const { getCloudinary, getCloudinaryUrl, createPublicId, configureCloudinary, updateHtmlImagesToCloudinary } = require('../../src/lib/cloudinary');
1+
const { ERROR_ASSET_UPLOAD, ERROR_INVALID_SRCSET } = require('../../src/data/errors');
2+
const { getCloudinary, createPublicId, getCloudinaryUrl, configureCloudinary, updateHtmlImagesToCloudinary } = require('../../src/lib/cloudinary');
23

34
const mockDemo = require('../mocks/demo.json');
45

@@ -69,7 +70,6 @@ describe('lib/util', () => {
6970
expect(cloudinaryUrl).toEqual(`https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/fetch/f_auto,q_auto/https://i.imgur.com/vtYmp1x.png`);
7071
});
7172

72-
// TODO: Mock functions to test Cloudinary uploads without actual upload
7373

7474
test('should create a Cloudinary URL with delivery type of upload from a local image', async () => {
7575
// mock cloudinary.uploader.upload call
@@ -95,6 +95,22 @@ describe('lib/util', () => {
9595
expect(cloudinaryUrl).toEqual(`https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/upload/f_auto,q_auto/stranger-things-dustin-fc571e771d5ca7d9223a7eebfd2c505d`);
9696
});
9797

98+
test('should fail to create a Cloudinary URL with delivery type of upload', async () => {
99+
// mock cloudinary.uploader.upload call
100+
cloudinary.uploader.upload = jest.fn().mockImplementation(() => Promise.reject('error'))
101+
102+
103+
await expect(getCloudinaryUrl({
104+
deliveryType: 'upload',
105+
path: '/images/stranger-things-dustin.jpeg',
106+
localDir: 'tests',
107+
remoteHost: 'https://cloudinary.netlify.app'
108+
})).rejects.toThrow(ERROR_ASSET_UPLOAD);
109+
});
110+
111+
112+
113+
98114
// TODO: Mock functions to test Cloudinary uploads without actual upload
99115

100116
// test('should create a Cloudinary URL with delivery type of upload from a remote image', async () => {

0 commit comments

Comments
 (0)