Skip to content
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

fix(gatsby-plugin-sharp): Don't swallow sharp errors #26806

Merged
merged 2 commits into from
Sep 8, 2020

Conversation

ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Sep 7, 2020

Currently sharp errors include the error message thrown by execa if a child process dies. However execa includes stdout and stderr in this message, and these may include image data in which case they fail to print, swallowing the error. See #26723 for example. This PR adds error.shortMessage if present, which is the error without stderr or stdout.

Before this PR:
image

After this PR:

image

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Sep 7, 2020
@ascorbic ascorbic added topic: sharp status: needs core review Currently awaiting review from Core team member and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Sep 7, 2020
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, would love to have a screenshot attached to the PR 😄

@gatsby-cloud
Copy link

gatsby-cloud bot commented Sep 7, 2020

Gatsby Cloud Build Report

client-only-paths

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 21s

Performance

Lighthouse report

Metric Score
Performance 💚 100
Accessibility 🔶 85
Best Practices 💚 100
SEO 🔶 70

🔗 View full report

@gatsby-cloud
Copy link

gatsby-cloud bot commented Sep 7, 2020

Gatsby Cloud Build Report

using-styled-components

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 20s

Performance

Lighthouse report

Metric Score
Performance 💚 100
Accessibility 💚 90
Best Practices 💚 100
SEO 💚 90

🔗 View full report

@gatsby-cloud
Copy link

gatsby-cloud bot commented Sep 7, 2020

Gatsby Cloud Build Report

using-reach-skip-nav

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 19s

Performance

Lighthouse report

Metric Score
Performance 💚 100
Accessibility 💚 100
Best Practices 💚 100
SEO 🔶 82

🔗 View full report

@ascorbic ascorbic added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Sep 7, 2020
@gatsby-cloud
Copy link

gatsby-cloud bot commented Sep 7, 2020

Gatsby Cloud Build Report

gatsby

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 20m

@gatsbybot gatsbybot merged commit 5549a23 into master Sep 8, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix/sharp-errors branch September 8, 2020 13:02
pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes status: needs core review Currently awaiting review from Core team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants