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-gatsby-cloud): add backpressure for IPC #32963

Merged
merged 2 commits into from
Sep 1, 2021

Conversation

vladar
Copy link
Contributor

@vladar vladar commented Aug 30, 2021

Description

Adds backpressure to IPC to prevent message loss in case of process exiting early. As mentioned in docs on process.send:

subprocess.send() will return false if the channel has closed or when the backlog of unsent messages exceeds a threshold that makes it unwise to send more. Otherwise, the method returns true. The callback function can be used to implement flow control.

Also related: nodejs/node#7657 (comment)

This PR also adds batching for emitRoute IPC calls.

P.S. GitHub diff here is pretty messy, use Files Changed -> Settings -> Hide whitespace characters to make it readable.

Caveat: Jest

Apparently, Jest sets up an incorrect stub for process.send which returns undefined: https://github.com/facebook/jest/blob/98f10e698ae986c19fef2d8117be2341bcfb8f7f/packages/jest-util/src/createProcessObject.ts#L113

Node always returns boolean from process.send (1, 2). To account for this - had to trick Jest by changing the original implementation to this.

Edit: submitted a PR to Jest with a proposed fix: jestjs/jest#11799

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Aug 30, 2021
@vladar vladar removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Aug 30, 2021
@vladar vladar merged commit eb1f568 into master Sep 1, 2021
@vladar vladar deleted the vladar/ipc-backpressure branch September 1, 2021 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants