fix: add possibility to use own handling of onerror which will not en…

[misino]

…d up in the exception

Description

Motivation and Context

We had a problem with generating an image when html contained pictures with src pointing to a domain which doesn't allow CORS.
This change is about possibility to specify own onerror callback which don't need to make whole image generation fail.

Types of changes

• 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 change)
• Enhancement (changes that improvement of current feature or performance)
• Refactoring (changes that neither fixes a bug nor adds a feature)
• Test Case (changes that add missing tests or correct existing tests)
• Code style optimization (changes that do not affect the meaning of the code)
• Docs (changes that only update documentation)
• Chore (changes that don't modify src or test files)

Self Check before Merge

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[biiibooo]

👋 @misino

💖 Thanks for opening this pull request! 💖

Please follow the contributing guidelines. And we use semantic commit messages to streamline the release process.

Examples of commit messages with semantic prefixes:

• fix: don't overwrite prevent_default if default wasn't prevented
• feat: add graph.scale() method
• docs: graph.getShortestPath is now available

Things that will help get your PR across the finish line:

• Follow the TypeScript coding style.
• Run npm run lint locally to catch formatting errors earlier.
• Document any user-facing changes you've made.
• Include tests when adding/changing behavior.
• Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 63.10%. Comparing base (b751cbf) to head (3b6fd62).
Report is 13 commits behind head on master.

Files with missing lines	Patch %	Lines
src/embed-images.ts	50.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #453      +/-   ##
==========================================
+ Coverage   62.93%   63.10%   +0.17%     
==========================================
  Files          10       10              
  Lines         580      580              
  Branches      143      143              
==========================================
+ Hits          365      366       +1     
  Misses        151      151              
+ Partials       64       63       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Imran-imtiaz48]

Review:

Summary:

This refactor significantly improves the error handling mechanism within the resourceToDataURL function. By replacing the inline async error handling with a more structured approach, it leverages a reusable error handler function. This change enhances code readability, maintainability, and reduces redundancy.

Positives:

1. Improved Readability: The introduction of options.onImageErrorHandler makes the code more modular and easier to understand.
2. Code Reusability: The error handler can be reused across different parts of the application, promoting DRY (Don't Repeat Yourself) principles.
3. Error Handling Enhancement: The structured try-catch block ensures that errors are caught and processed consistently.

Suggestions for Improvement:

1. Test Coverage: The added line PNG image rendering as blank on IOS 13 view option #66 is not covered by tests. To ensure robustness, consider adding test cases that trigger this error handler to verify its functionality.
2. Documentation: Adding comments or documentation about the options.onImageErrorHandler would help other developers understand its purpose and usage more quickly.
3. Fallback Mechanism: In the absence of options.onImageErrorHandler, you might want to ensure there's a default error handling mechanism to prevent potential runtime errors if onImageErrorHandler is not provided.

Code Snippet for Additional Tests:

describe('Error Handling in resourceToDataURL', () => {
  it('should call the onImageErrorHandler when an error occurs', async () => {
    const mockHandler = jest.fn();
    const options = {
      onImageErrorHandler: mockHandler,
    };
    
    // Assuming resourceToDataURL is the function being tested
    await resourceToDataURL('invalid_url', options);
    
    expect(mockHandler).toHaveBeenCalled();
  });
  
  it('should reject with an error if no onImageErrorHandler is provided', async () => {
    const options = {};
    
    await expect(resourceToDataURL('invalid_url', options)).rejects.toThrow('Image failed to load');
  });
});

---

Great job on the refactor! With these minor improvements, the codebase will be even more robust and maintainable.