Skip to content

[Feature] Log X-Cld-Error header in dev mode #512

Closed

Description

@colbyfayock

Feature Request

Is your feature request related to a problem? Please describe.

Cloudinary returns an X-Cld-Error header on image error that typically includes something descriptive of the reason the error is failing.

To help debugging, we can log this error in to the console, but only on dev mode.

This would need to happen from within the onError handling where we currently poll for the image for handling images that are processing.

Activity

  1. ayan-joshi commented on Oct 3, 2024

    @ayan-joshi

    @colbyfayock Hey can I work on this issue ?

  2. colbyfayock commented on Oct 3, 2024

    @colbyfayock
    CollaboratorAuthor

    all yours! let me know if you have any questions when trying to tackle it or if anything is unclear

  3. ayan-joshi commented on Oct 3, 2024

    @ayan-joshi

    @colbyfayock I wanted to know the location of file which logs X-Cld-Error

  4. colbyfayock commented on Oct 3, 2024

    @colbyfayock
    CollaboratorAuthor

    this is where the error occurs and the polling check: https://github.com/cloudinary-community/next-cloudinary/blob/main/next-cloudinary/src/components/CldImage/CldImage.tsx#L99

    however the polling that looks at the request response is inside: https://github.com/cloudinary-community/cloudinary-util/blob/4086730a5186b142a4c9afc15c1b199a3f738935/packages/util/src/lib/cloudinary.ts#L163

    it would need to be a breaking change, but the solution may be to return information about the response in the catch statement

    something along the lines of:

    return {
       error: true,
       status: response.status,
       cldError: resp.headers.get('x-cld-error'))
    }
    

    wdyt?

  5. ayan-joshi commented on Oct 3, 2024

    @ayan-joshi

    @colbyfayock If we want error statement in console only we can show it with console for the devs , or we can use toast with the statement for both devs and users to show it in the interface what do we want

  6. ayan-joshi commented on Oct 3, 2024

    @ayan-joshi
    try {
        const response = await fetch(options.src);
    
        if (response.status === 423) {
          await new Promise((resolve) => setTimeout(resolve, 500));
          return await pollForProcessingImage(options);
        }
        if (!response.ok) {
          return {
            error: true,
            status: response.status,
            cldError: response.headers.get('x-cld-error') || 'Unknown error',
          };
        }
    
        return { error: false };
      } catch (error) {
        return {
          error: true,
          status: 500,
          cldError: (error as Error).message || 'Network error',
        };
      }
    

    @colbyfayock I feel like this should be the format of try catch with response ! ok between them for all type of cases

  7. colbyfayock commented on Oct 7, 2024

    @colbyfayock
    CollaboratorAuthor

    @ayan-joshi i think that solution makes sense! we could event return the following for the success path:

    return { status: response.status };
    

    not sure the error as false is absolutely necessary but ill leave tht to your judgement

    could you open a PR for that in the cloudinary-util repository?

  8. ayan-joshi commented on Oct 7, 2024

    @ayan-joshi

    @ayan-joshi i think that solution makes sense! we could event return the following for the success path:

    return { status: response.status };
    

    not sure the error as false is absolutely necessary but ill leave tht to your judgement

    could you open a PR for that in the cloudinary-util repository?

    @colbyfayock okay sure I'll make the changes and create the pr for it ,
    i need to create pr in both cloudinary-util and cloudinary-next right ??

  9. colbyfayock commented on Oct 8, 2024

    @colbyfayock
    CollaboratorAuthor

    hey @ayan-joshi the change was published to @cloudinary-util/util@4.0.0-beta.1

    can you try upgrading that version on a branch from this repository and then you can integrate the new changes? you can then inspect the response, look for the error, and log it only in development mode

    let me know if you run into any issues!

  10. ayan-joshi commented on Oct 8, 2024

    @ayan-joshi

    okay I'll try it and respond @colbyfayock

  11. 2 remaining items

  12. colbyfayock commented on Oct 11, 2024

    @colbyfayock
    CollaboratorAuthor

    no worries! just checking in on Issues to make sure people are still interested. thanks for the heads up!

  13. colbyfayock commented on Oct 15, 2024

    @colbyfayock
    CollaboratorAuthor

    @ayan-joshi i needed to merge in the beta to a stable release and htus needed to make this work, so handling this myself

  14. github-actions commented on Oct 15, 2024

    @github-actions

    🎉 This issue has been resolved in version 6.15.0 🎉

    The release is available on:

    Your semantic-release bot 📦🚀

  15. ayan-joshi commented on Oct 15, 2024

    @ayan-joshi

    @colbyfayock really sorry that I couldn't make it with the time !

  16. added a commit that references this issue on Oct 15, 2024
    711c759
  17. colbyfayock commented on Oct 15, 2024

    @colbyfayock
    CollaboratorAuthor

    no worries! feel free to pick up another one if interested when you have time

  18. ayan-joshi commented on Oct 15, 2024

    @ayan-joshi

    no worries! feel free to pick up another one if interested when you have time

    Yeah sure I'll

  19. added a commit that references this issue on Oct 15, 2024
  20. github-actions commented on Oct 15, 2024

    @github-actions

    🎉 This issue has been resolved in version 7.0.0-beta.4 🎉

    The release is available on:

    Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions