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): Retry stalled remote file downloads #20843

Merged
merged 8 commits into from
Jan 31, 2020

Conversation

ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Jan 24, 2020

Description

gatsby-source-filesystem has a problem where downloads would stall and never timeout or error. Eventually all the concurrent downloads were stalled, preventing any more starting. This was causing builds to stall indefinitely.

This PR adds a timeout in createRemoteFileNode. If no data events are received in the download stream for 30 seconds then the download is cancelled and retried up to three times. In tests this unblocks the downloads on the first attempt.

WIP while I'm writing unit tests.

Documentation

Related Issues

@ascorbic ascorbic marked this pull request as ready for review January 27, 2020 17:08
@ascorbic ascorbic requested a review from a team as a code owner January 27, 2020 17:08
@pieh pieh self-assigned this Jan 27, 2020
@ascorbic ascorbic requested a review from pieh January 29, 2020 17:26
Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

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

Thank you! 🥇

Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

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

Left a small comment!

@sidharthachatterjee sidharthachatterjee added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Jan 31, 2020
@gatsbybot gatsbybot merged commit 536686b into master Jan 31, 2020
@delete-merged-branch delete-merged-branch bot deleted the retry-stalled-downloads branch January 31, 2020 12:37
})
const fsWriteStream = fs.createWriteStream(tmpFilename)
responseStream.pipe(fsWriteStream)
responseStream.on(`downloadProgress`, pro => console.log(pro))
Copy link
Contributor

@muescha muescha Feb 15, 2020

Choose a reason for hiding this comment

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

the console.log is removed accidentally in resetTimeout?

@mjmaurer
Copy link
Contributor

mjmaurer commented Apr 15, 2020

I think this is breaking my build (at least locally with gatsby develop).


Don't have time to grok the code, but what I think is happening:

I'm using gatsby-source-contentful with downloadLocal enabled. When createRemoteFileNode is called here, the entire batch of images is being given the same timeout (30 seconds in this PR). I'm guessing the timeout is somehow not being cleared.


Ultimately, what I'm sure of is that increasing the timeout to 20 min fixes my issues.

mwfrost pushed a commit to mwfrost/gatsby that referenced this pull request Apr 20, 2023
…atsbyjs#20843)

* fix(gatsby-source-filesystem): Retry stalled remote file downloads

* Switch to got@8

* Await the recursive callback

* Add tests for loading fix

* Don't reset on data. Start timout after `response`

* Add got timeout

* Don't await the recursive call

* Use constants for connection timeouts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants