-
Notifications
You must be signed in to change notification settings - Fork 8
Catch Pixelmatch error for mismatched images sizes #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Add try/catch to handle image size mismatch errors Update error message to reflect mismatch error & not log non-existent diff path
revert formatting
index.js
Outdated
| if(e.message !== IMAGE_SIZE_ERROR) console.error(e) | ||
| reject({ | ||
| errorPixelCount, | ||
| errorPixelCount: Math.abs((baseImg.data.length - tmpImg.data.length) / 4), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? 🤔 Can you explain this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah I see, this is virtual here basically.. Can you rewrite this to make this clearer to something like this:
} catch (e) {
if(e.message === IMAGE_SIZE_ERROR) {
return reject(...);
}
reject(e);
}Currently this would swallow other errors, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
image.data.length is the number of pixels x 4 (for RGBA channels); within the version of Pixelmatch this is using, it verifies that the image data is exactly width x height x 4.
Taking the absolute value will return a positive count of differing pixels, regardless of which image is smaller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change around rejection is sound, and I think the error should be logged so the user can see what unexpected behaviour is there
| data.fullDiffPath = diffPath? path.join(__dirname, diffPath) : null; | ||
|
|
||
| if(reason.diffPath === null){ | ||
| data.error = `Image sizes do not match ${errorPixelCount} pixels differ - img: ${tmpPath}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| data.error = `Image sizes do not match ${errorPixelCount} pixels differ - img: ${tmpPath}`; | |
| data.error = `Image sizes do not match - ${errorPixelCount} pixels differ - img: ${tmpPath}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just copying the formatting of the existing error message
Throw error on size match error & reflect same in error message