[Bug] Error when using secureDistribution and privateCDN instead of cloudName

[cprussin]

Bug Report

Error thrown when setting cloud name via props instead of using env vars.

Describe the bug

Using CldImage like this:

<CldImage config={{ url: { secureDistribution: "foo", privateCdn: true }, cloud: { cloudName: "" } }} ... />

is valid and works correctly (however setting cloudName to an empty string is necessary, see cloudinary/js-url-gen#588), however due to #353 now causes an exception (Error: A Cloudinary Cloud name is required ...) to be thrown.

The fix is to set either NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME or config.cloud.cloudName to some dummy value, even though the value isn't actually used anywhere.

Is this a regression?

Yes, regression was added in #353

Steps To Reproduce the error

1. Create a component like <CldImage config={{ url: { secureDistribution: "foo", privateCdn: true }, cloud: { cloudName: "" } }} ... />
2. Don't set NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME
3. ???
4. Profit!

Expected behaviour

No exception if the config props contain the necessary configs (i.e. only throw an exception if both NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME is missing and the config prop is missing the necessary values)

[colbyfayock]

hey @cprussin im having a hard time reproducing this on latest

i removed my environment variable and with my logs and changes shown below, im able to see the values, and the images load in my browser

<CldImage
      ...
      config={{
        cloud: {
          cloudName: 'colbycloud-next-cloudinary'
        }
      }}
    />

i tried adding your url config as well just to see if that was triggering an issue and the links broke of course but it still loaded without the error

will keep looking at this after some meetings and see if im missing something

[cprussin]

ah sorry @colbyfayock I should have been more clear, the bug is actually triggered by explicitly leaving cloudName as an empty string, and using secureDistribution and privateCDN.

If you look at https://github.com/cloudinary/js-url-gen/blob/e1b97191fff165b6ecb353acb2477cc25a062667/src/assets/CloudinaryFile.ts#L182-L193, there must be a value for cloudName or the underlying cloudinary lib blows up. However, if you look at https://github.com/cloudinary/js-url-gen/blob/e1b97191fff165b6ecb353acb2477cc25a062667/src/internal/url/cloudinaryURL.ts#L20-L51, the cloudName isn't actually used if using secureDistribution and privateCDN. This is certainly a bug in @cloudinary/url-gen and I'll put in a bug report there, but also this lib shouldn't force a cloud name in cases where it isn't actually used.

[colbyfayock]

ohh you know i noticed that empty string but i was thinking it was just from the example you shared

will get this addressed but yes please, if you dont mind, creating an issue for URL-gen as well

[cprussin]

url-gen issue: cloudinary/js-url-gen#588

[cprussin]

@colbyfayock on further thought you probably shouldn't touch this lib until the url-gen ticket gets resolved, as presumably any error messaging you add here should be consistent with url-gen's underrlying error messaging

[colbyfayock]

i think that makes sense however I'm just not sure of the timeline of them getting that fixed, but i can get mine fixed right away by adding a more explicit undefined check

if ( typeof cloudName === 'undefined' )

while an empty string shouldnt technically be a valid value, i would prefer the library to be in a working state even if it requires a little hack like you have, and i can unblock you for that

[cprussin]

Up to you -- I wouldn't worry too much about it as it's trivial to work around by just setting cloudName to some dummy string like "-", which is honestly no less semantically valid than passing an empty string for an unused value 😂 .

If anything maybe this should just be a doc change between now and when, if ever, it's fixed in the underlying.