Gatsby-source-wordpress-experimental: Gatsby develop build fails when sourcing an image that returns a HTTP code 500

[joesanchezjr]

At the moment, if an image returns a 500, the whole build fails and prevents further development.

This kind of bug pretty much prevents building for production which would not be ideal for sites that are media heavy like the one I'm working on.

I think I would rather have a missing image on my site (that I am aware of because of a warning either in the command line or as a graphql error) than not be able to develop my website at all.

Expected behavior

A warning similar to when an image returns an HTTP code 404, which currently looks like:

warn  gatsby-source-wordpress

Received a 404 when trying to fetch {IMAGE URL}
from Post #12490 {POST TITLE}

Most likely this image was uploaded to this Post and then deleted from the media library.
You'll need to fix this and re-save this Post to remove this warning at
{POST EDIT URL}

This gives the developer an idea as to what images needs to be addressed without preventing them from developing further.

Actual Behavior

ERROR 

gatsby-source-wordpress  failed to process {IMAGE_URL}
HTTPError: Response code 500 (Internal Server Error)

...

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] develop: `gatsby develop`
npm ERR! Exit status 1

[TylerBarnes]

Thanks for this suggestion @joesanchezjr , I've extended the error message info to apply to all errors in [email protected].

[joesanchezjr]

I received a different error about saying "cannot read property includes of undefined" and I believe it's this line:

https://github.com/gatsbyjs/gatsby-source-wordpress-experimental/blob/master/src/steps/source-nodes/create-nodes/process-node.js#L726

I changed it and was able to continue, but then I saw the same error I saw earlier.

Edit: I'll try to create a reproducible test case for you, so that you can see the error I see.

[TylerBarnes]

@joesanchezjr do you have a /graphql endpoint I can use to reproduce? That'd help here :) thanks!

[joesanchezjr]

@TylerBarnes Sure thing. The endpoint we are using is https://develop.preemptivelove.org/graphql. I had to limit the schema.perPage to 15 and the type.Post.limit to 500 in the gatsby-config file. If this is enough for you to work with, awesome! If not, let me know so that I can try to create a repo that has this error for you.

[TylerBarnes]

So I wasn't able to reproduce with your endpoint unfortuntely. A 500 error generally means your server is being overwhelmed by requests which is hard to reproduce if it doesn't happen everytime. However, I did add a null value check to the part of the code you mentioned getting that error in. Try updating to [email protected] to see if it works

[joesanchezjr]

I updated to the latest version and cleared my cache. I think the issue is resolved for now. The process is no longer exiting, but I still see errors in the console. Thanks for your help!

[joesanchezjr]

Update: the error is actually not resolved - I'm not sure why it wasn't thrown earlier. I'll continue to investigate. I've cloned the source plugin and will troubleshoot some more locally since I have so far been unsuccessful with creating a reproducible test case.

[joesanchezjr]

It looks like this is the part of the code that is exiting the process - specifically line 264 when running helpers.reporter.panic()

https://github.com/gatsbyjs/gatsby-source-wordpress-experimental/blob/d11188149530cf58e4e9a643d737f542321473a6/src/steps/source-nodes/create-nodes/process-node.js#L252-L266

I was able to get around it by changing that line to .warn instead of .panic and by adding imageNode = null after. This allowed the source plugin continue sourcing while also logging the error in the console.

Would it be possible to add an option to the plugin so that this line doesn't exit the process - similar to the newly added failOnError option added to gatsby-plugin-sharp?

It looks like at the moment, the plugin will fail a build if an image is missing. As I'm building out my new site, I've considered if this is something that should be optional. It could be that a site with 1000 photos is able to successfully source 999 photos, but not one. Exiting the build process seems like a lot for something that would conceivable by fixed by resolving the issue with the image via the WordPress admin. I'd liked to defer to you since you know more of the other processes happening with the plugin and how this potential change would affect them. What do you think @TylerBarnes ?

Also the following code is not what was exiting the gatsby develop process, but it seems like it might be related and would be the part that breaks the gatsby build process.

https://github.com/gatsbyjs/gatsby-source-wordpress-experimental/blob/d11188149530cf58e4e9a643d737f542321473a6/src/steps/source-nodes/create-nodes/create-remote-media-item-node.js#L45-L75

[TylerBarnes]

@joesanchezjr , the code you mentioned will allow the build to not fail in development for missing images (404), for other errors (5xx), this is usually due to your server being overwhelmed by requests rather than missing images. We'll be improving the error message and docs for this situation (see https://github.com/gatsbyjs/gatsby-source-wordpress-experimental/issues/85). I think that issue might encompass this one but I'm curious what you think. I'll leave this one open for now in case I'm misunderstanding. Let me know :)

[joesanchezjr]

I think improved error messaging and docs would be incredibly helpful! And I'm not sure if this is part of the plan, but providing the developer the option to allow a completed build even with missing images or server errors would be even more helpful. I'm not sure I understand the reasoning for a build failure if there isn't something technically wrong in the code, but on the server (and in this case WP) - which could theoretically be solved by triggering a new build once the media is fixed on the WP/server side.

I think my particular issue (although closely related to the issue you linked) is more so asking for a solution to the part that _exits the build process_ on a media sourcing error and less so asking for better messaging. I see how my original comments might seem to mean otherwise, but does that make sense?

[TylerBarnes]

@joesanchezjr , the reason it should fail the build is that there is incomplete source data. Say for example your site has 20,000 images and there's a server problem so the second 10k aren't sourced, that means the site will deploy with 10k missing images 😱
I do agree this should be allowed in development though. 🤔

[joesanchezjr]

@TylerBarnes true - I understand that concern now. I guess my concern was for a different rate of failure. For example, a site with 20k images but only 10 images aren't sourced. I think that's where the good error messaging and the developer _option_ to ignore the errors (even for production) comes in. Failing on error for production is still a good default, but having the ability to not fail on error is, at least I think, a net good _option_.

Also side note, really appreciate the time you've taken with me on this 😄

[TylerBarnes]

For sure :) same here! My line of thinking is that there aren't any acceptable errors in production. We could for example offer an option where you could set a percentage of acceptable failures, but we're intending to make this plugin as easy to use and as resilient as normal WP (or more so). I think rather than allowing missing data, we should improve our algorithm to prevent the failures that happen due to overwhelming the WP server with requests. In development I agree that failures are ok though 👍

The product manager working with me on this project (@jacksellwood) and I were talking about how we could even potentially have an all images release in the future where we address many of the difficult aspects of working with WP images in Gatsby. I could see a feature like this going into that release for sure.

[joesanchezjr]

I can see where the caution is coming from. I do like the rate of failure idea too - I was considering mentioning that. Errors on production are definitely not a good thing, but I would imagine that then if a developer _chose_ to turn on the option that prevents failing on errors, then it would fall on them to resolve those errors on the front end since they have chosen something that is not the default.

For example:

I have 100 blog posts, all with a featured image. 99 of those images are sourced properly, 1 is not. I decide to use the option that allows the build to continue even when that image is missing.

In my code, I write a check that would look similar to -

if (imageExistsBecauseItWasSourcedCorrectly) {
    displayImage()
} else { 
    displayBackUpImage()
}

So now I can successfully build my site and in the few instances where the images weren't sourced correctly, a user will load the page where they won't get a broken image but the default one - and technically speaking the client doesn't have any errors, only the server/WP does.

Also to clarify, I'm actually not getting the 500 error because of too many requests to my server. There is actually something wrong with how the image is being served. Which, granted, needs to be addressed on its own, but I still think that the way these are errors are handled via this plugin should have options.

Edit: I think I keep wanting to be able to fix this on the front end because of the headless nature of using Gatsby and WordPress. I want to be able to get the data (both valid and invalid) and _then_ deal with the data with my front-end code (including any errors). So I think where we are differing in view is how we decide where we want to handle the errors.

[TylerBarnes]

@joesanchezjr , that makes sense to want that. The reason it's good to have the build fail is because you'll know when things go wrong that way, but you wont if you put your error handling in templates. Failing the build is actually a feature that means your site content wont slowly degrade without you realizing it. When a build succeeds it will have valid data, then in the future when something goes wrong you'll know immediately, not when you happen to see that a template is missing an image

[rickgregory]

@TylerBarnes - I'm with @joesanchezjr on this. Warn us, but don't force the build to fail. It's more than a little annoying to have a plugin coded to that it removes agency from us, the developers. I fully understand your outlook, but what you're doing is making a decision for us vs giving us information and then allowing us to decide how we want to handle it. Please reconsider.

[TylerBarnes]

@rickgregory I hear that for sure! I mentioned above ☝️ that we've been talking about having a release just for images in the future and we'll consider adding this at that point. Thanks for chiming in :)

[TylerBarnes]

Also if either of y'all are interested in opening a PR that adds a plugin option which allows this, that'd be helpful too!

[TylerBarnes]

As of [email protected] there's now a plugin option to allow 404's in production builds. For now we aren't extending this to other error codes, but PR's are always welcome. Once you upgrade, the 404 error message will show you how to use the new plugin option.

Thanks everyone!