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-source-filesystem,gatsby-transformer-sharp): Use custom errors #27576

Merged
merged 1 commit into from
Oct 26, 2020
Merged

fix(gatsby-source-filesystem,gatsby-transformer-sharp): Use custom errors #27576

merged 1 commit into from
Oct 26, 2020

Conversation

ax-vasquez
Copy link
Contributor

@ax-vasquez ax-vasquez commented Oct 20, 2020

Description

Related to Cloud functionality

In effort to improve the Cloud user experience, we need to make sure "Error copying file from..." errors are appropriately captured/categorized for Builds and Previews.

This PR simply updates the locations where log this error so that it uses reporter.panic instead of console.error

To do

  1. What id should be used for this error? - Any 5-digit number; this is an arbitrary value and contained within the context of the plugin (e.g., there is no risk error code conflicts between modules)
    • Should the id be different between gatsby-source-filesystem and gatsby-transformer-sharp (the two places that log this error) - Yes, the module name is included in the ID prefix for both error locations.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 20, 2020
@LekoArts LekoArts added topic: plugins Related to plugin system, themes & catch-all for plugins that don't have a label and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 21, 2020
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

Left some (hopefully) helpful comments. For reference, this was the PR adding the same logic in another plugin: #27441

packages/gatsby-source-filesystem/src/extend-file-node.js Outdated Show resolved Hide resolved
packages/gatsby-source-filesystem/src/extend-file-node.js Outdated Show resolved Hide resolved
packages/gatsby-transformer-sharp/src/customize-schema.js Outdated Show resolved Hide resolved
packages/gatsby-transformer-sharp/src/customize-schema.js Outdated Show resolved Hide resolved
@ax-vasquez ax-vasquez requested a review from LekoArts October 21, 2020 17:14
@ax-vasquez ax-vasquez marked this pull request as ready for review October 21, 2020 17:20
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

Two more smaller requests, rest looks good

packages/gatsby-source-filesystem/src/error-utils.js Outdated Show resolved Hide resolved
packages/gatsby-transformer-sharp/src/error-utils.js Outdated Show resolved Hide resolved
@ax-vasquez ax-vasquez requested a review from a team as a code owner October 23, 2020 15:06
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

👍

@LekoArts LekoArts changed the title Use reporter.panic call when unable to copy files instead of console.error fix(gatsby-source-filesystem,gatsby-transformer-sharp): Use custom errors Oct 26, 2020
@LekoArts LekoArts merged commit 752e27a into gatsbyjs:master Oct 26, 2020
@ax-vasquez ax-vasquez deleted the armandovasquez/ch17057/useReporterForUnableToCopyError branch October 26, 2020 17:39
mwfrost pushed a commit to mwfrost/gatsby that referenced this pull request Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: plugins Related to plugin system, themes & catch-all for plugins that don't have a label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants