fix(gatsby-source-filesystem): Fix timing issue - processing nodes while flushing

[Andarist]

Followup to #17192 (comment)

Description

This PR aims to solve a minor timing issue. Right now nodes can be created/changed/removed instantly while flushing path queue, so even before a returned promise settles. It has been suggested by @pieh that this might be a slight problem, although surely an edge case.

Before

After

As you might notice on the After visualization paths queuing seems to be moved to the outside of CHOKIDAR state so it might look like those events always lead to queuing. This is not the case though, I've leveraged the fact of how XState works - we can "override" outer transitions by specifying inner ones. So actually we always queue (in NOT_READY & FLUSHING) but we process in READY (because we've overridden queuing there). This is somewhat less explicit, especially for people not familiar with XState but it makes the visualization more readable (it declutters it). If you don't like it I can change to more "explicit" form of this - by duplicating queuing transitions on both NOT_READY & FLUSHING states.

This is still WIP because we need to establish how flushing errors should be handled. Any thoughts @pieh ?

[pieh]

Let me do quick comment first - please don't investigate checks error - this is 100% unrelated to this PR and is being worked on separately (some context in #17515 )

I'm thinking that those errors should be handled earlier, so it's just not possible for flushPathQueue service to fail. I'm not sure what's recommended approach in cases like that - should we skip onError or implement it with same handling as onDone for completeness

I'll think about error handling but initial thought is that most likely only error that can happen is I/O related - particularly in

gatsby/packages/gatsby-source-filesystem/src/create-file-node.js

Line 26 in 8562e93

const stats = await fs.stat(slashedFile.absolutePath)

I imagine weird edge case when this in particular could happen if we would prep synthetic test that would start watching files, then create file and delete it before chokidar's ready event which would queue addition and deletion for same file, and during events flushing, addition would error out because fs.stat on non-existing file does result in error.

Not exhaustive list of I/O errors that can happen is:

• EACCES (Permission denied): An attempt was made to access a file in a way forbidden by its file access permissions.
• EMFILE (Too many open files in system): Maximum number of file descriptors allowable on the system has been reached, and requests for another descriptor cannot be fulfilled until at least one has been closed. This is encountered when opening many files at once in parallel, especially on systems (in particular, macOS) where there is a low file descriptor limit for processes. To remedy a low limit, run ulimit -n 2048 in the same shell that will run the Node.js process.
• ENOENT (No such file or directory): Commonly raised by fs operations to indicate that a component of the specified pathname does not exist — no entity (file or directory) could be found by the given path.

I don't think there's good way of handling EACCES error other than displaying error and terminating build. For ENOENT maybe retry mechanism and ignoring it after Nth unsuccessful attempt? The EMFILE is interesting and will need to think what's feasible way to handle it.

[muescha]

CHOKIDAR.READY and CHOKIDAR_READY Is hard to distinct. And the next one debugging is wondering why point or interline or is this an error. Better a litle more different names.

[Andarist]

Good point, I'll think about a differentiating them.

[Andarist]

Let me do quick comment first - please don't investigate checks error - this is 100% unrelated to this PR and is being worked on separately (some context in #17515 )

Good to know, was about to investigate those 😅

As to error handling - just reassuring myself if I understand you correctly. Do you propose handling flushing errors downstream (i.e. inside createFileNode) and bubbling only really terminal ones to the flushPathQueue (like EACCESS)? This sounds reasonable to me.

One concern - if we plan to terminate the build in any case while flushing the queue then I feel like similar thing should happen here:

gatsby/packages/gatsby-source-filesystem/src/gatsby-node.js

Lines 99 to 104 in 8562e93

	createAndProcessNode(_, { pathType, path }, { state }) {
	if (state.matches(`BOOTSTRAP.BOOTSTRAPPED`)) {
	reporter.info(`added ${pathType} at ${path}`)
	}
	createAndProcessNode(path).catch(err => reporter.error(err))
	},

[Andarist]

Just want to let you know that I'm going to definitely work on this more, just didn't have time lately to get back to it.

[pieh]

Just want to let you know that I'm going to definitely work on this more, just didn't have time lately to get back to it.

That's perfectly ok! Thanks for letting us know

[sidharthachatterjee]

Hey @Andarist

Any chance you've had a moment to look at this? We're doing some spring cleaning and were wondering what the status here is. Happy to jump on a call and pair with you if you like 🙂

[Andarist]

@sidharthachatterjee Hi! I was actually thinking about this one this week and was planning to drop by here to leave a note that I have not forgotten about this, I just have very little time to finish this right now. I currently work on other OSS stuff which occupies all of my free time.

I hope to get back to this month - as from what I recall there isn't that much leftover work to do here. I might take you up on that offer then :p

[pvdz]

(I've converted this PR into a Draft PR. I hope you're still thinking of this :) )

[Andarist]

Yes, I'm still thinking about this - weekly 😅I'm afraid that my life has changed quite a bit lately and I just need to learn to focus on things, instead of trying to send as many PRs to as many OSS projects as I can 😉At the moment I'm focusing on improving XState itself and I just couldn't find the time finish this PR - I still have every intention to get back to it someday, but I can't say when that could be. If anyone wants to take a stab at this - feel free to pick it up in the meantime, I'm always up for discussing things, reviewing etc, so if one wants to learn some XState while working on this one I can provide support and guidance.

[LekoArts]

Appreciate the PR but as it got stale I'll close this one. Please reach out to us if you want to continue it or send in something similar. Thanks!