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

[gatsby-source-filesystem] errors are ignored in createRemoteFileNode #6643

Closed
HriBB opened this issue Jul 21, 2018 · 9 comments · Fixed by #10123
Closed

[gatsby-source-filesystem] errors are ignored in createRemoteFileNode #6643

HriBB opened this issue Jul 21, 2018 · 9 comments · Fixed by #10123
Labels
help wanted Issue with a clear description that the community can help with.

Comments

@HriBB
Copy link

HriBB commented Jul 21, 2018

Took me a while to find a bug in my sourceNodes code, because errors are ignored in createRemoteFileNode inside processRemoteNode function.

I kept getting null back from createRemoteFileNode. So after some digging, I put a console.log directly into gatsby-source-filesystem inside my node_modules and see the error

error TypeError: createNodeId is not a function
    at /home/bojan/www/instaset/website/node_modules/gatsby-source-filesystem/create-file-node.js:61:11
    at Generator.next (<anonymous>)
    at step (/home/bojan/www/instaset/website/node_modules/@babel/runtime/helpers/asyncToGenerator.js:12:30)
    at _next (/home/bojan/www/instaset/website/node_modules/@babel/runtime/helpers/asyncToGenerator.js:27:9)
    at run (/home/bojan/www/instaset/website/node_modules/core-js/library/modules/es6.promise.js:75:22)
    at /home/bojan/www/instaset/website/node_modules/core-js/library/modules/es6.promise.js:92:30
    at flush (/home/bojan/www/instaset/website/node_modules/core-js/library/modules/_microtask.js:18:9)
    at process._tickCallback (internal/process/next_tick.js:61:11)

Maybe we can do something about that?

@KyleAMathews
Copy link
Contributor

Is this for a custom source plug-in?

@HriBB
Copy link
Author

HriBB commented Jul 21, 2018

Yes.

@KyleAMathews
Copy link
Contributor

This is probably what you need https://next.gatsbyjs.org/docs/migrating-from-v1-to-v2/#createremotefilenode

@HriBB
Copy link
Author

HriBB commented Jul 22, 2018

Hmm ... I didn't present this problem in a correct way :) I know about the new createNodeId parameter. But I made a mistake and imported it from actions instead of props

exports.sourceNodes = async ({ actions, store, cache }) => {
  const { createNode, createNodeId } = actions
  // ...
}

createRemoteFileNode kept returning null, and there was no error, because it is ignored. Is this intentional?

I already solved my problem, you can close this issue if you wish. Just wanted to point this out ;)

This is the right way to destructure props:

exports.sourceNodes = async ({ actions, createNodeId, store, cache }) => {
  const { createNode } = actions
  // ...
}

@KyleAMathews
Copy link
Contributor

Yeah, validating the options and throwing an error sounds like a great plan. Would you like to PR that?

@m-allanson m-allanson added help wanted Issue with a clear description that the community can help with. 🏷 type: feature labels Jul 24, 2018
@HriBB
Copy link
Author

HriBB commented Jul 25, 2018

Sure. I'm rather busy ATM, but will try to find some time to look at the situation ;)

@HriBB
Copy link
Author

HriBB commented Aug 2, 2018

@KyleAMathews I looked at the code a little bit. The error is ignored in the processRemoteNode function:

async function processRemoteNode({
  url,
  store,
  cache,
  createNode,
  auth = {},
  createNodeId,
}) {
  try {
    // ...
  } catch (err) {
    // ignore <<<<<<<<<<<<<<<< HERE
  }
  return null
}

We could throw err there, but then what's the point of even having a try catch block? Or did you mean that we should throw in options are invalid?

Are there any examples on how to validate the options, or should we simply check for the presence of all required parameters such as createNodeId function for example?

@mglaman
Copy link
Contributor

mglaman commented Oct 26, 2018

I just ran into this as well because of some 404s. The silent failure made this pretty hard to debug when using gatsby-source-drupal.

Haroenv added a commit to Haroenv/gatsby that referenced this issue Nov 25, 2018
fixes gatsbyjs#6643

I also fell for this error before, sinc the original errors are swallowed (and fairly unclear too, because of the indirection). Adding validation was suggested in linked issue, so I went for this, since I didn't see any already used object validation like "ow" in use here.
pieh pushed a commit that referenced this issue Nov 26, 2018
…10123)

fixes #6643

I also fell for this error before, sinc the original errors are swallowed (and fairly unclear too, because of the indirection). Adding validation was suggested in linked issue, so I went for this, since I didn't see any already used object validation like "ow" in use here.

<!--
  Q. Which branch should I use for my pull request?
  A. Use `master` branch (probably).

  Q. Which branch if my change is a bug fix for Gatsby v1?
  A. In this case, you should use the `v1` branch

  Q. Which branch if I'm still not sure?
  A. Use `master` branch. Ask in the PR if you're not sure and a Gatsby maintainer will be happy to help :)

  Note: We will only accept bug fixes for Gatsby v1. New features should be added to Gatsby v2.

  Learn more about contributing: https://www.gatsbyjs.org/docs/how-to-contribute/
-->
gpetrioli pushed a commit to gpetrioli/gatsby that referenced this issue Jan 22, 2019
…atsbyjs#10123)

fixes gatsbyjs#6643

I also fell for this error before, sinc the original errors are swallowed (and fairly unclear too, because of the indirection). Adding validation was suggested in linked issue, so I went for this, since I didn't see any already used object validation like "ow" in use here.

<!--
  Q. Which branch should I use for my pull request?
  A. Use `master` branch (probably).

  Q. Which branch if my change is a bug fix for Gatsby v1?
  A. In this case, you should use the `v1` branch

  Q. Which branch if I'm still not sure?
  A. Use `master` branch. Ask in the PR if you're not sure and a Gatsby maintainer will be happy to help :)

  Note: We will only accept bug fixes for Gatsby v1. New features should be added to Gatsby v2.

  Learn more about contributing: https://www.gatsbyjs.org/docs/how-to-contribute/
-->
@antoinerousseau
Copy link
Contributor

fixed in #12348

mwfrost pushed a commit to mwfrost/gatsby that referenced this issue Apr 20, 2023
…atsbyjs#10123)

fixes gatsbyjs#6643

I also fell for this error before, sinc the original errors are swallowed (and fairly unclear too, because of the indirection). Adding validation was suggested in linked issue, so I went for this, since I didn't see any already used object validation like "ow" in use here.

<!--
  Q. Which branch should I use for my pull request?
  A. Use `master` branch (probably).

  Q. Which branch if my change is a bug fix for Gatsby v1?
  A. In this case, you should use the `v1` branch

  Q. Which branch if I'm still not sure?
  A. Use `master` branch. Ask in the PR if you're not sure and a Gatsby maintainer will be happy to help :)

  Note: We will only accept bug fixes for Gatsby v1. New features should be added to Gatsby v2.

  Learn more about contributing: https://www.gatsbyjs.org/docs/how-to-contribute/
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issue with a clear description that the community can help with.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants