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 requested review from pieh and wardpeet January 24, 2020 17:33
@ascorbic ascorbic force-pushed the retry-stalled-downloads branch from df34bb6 to d673e9a Compare January 24, 2020 17:35
@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 force-pushed the retry-stalled-downloads branch from 837654d to 5304fc9 Compare January 29, 2020 15:22
@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!

@ascorbic ascorbic force-pushed the retry-stalled-downloads branch from 56d11bd to f02a80c Compare January 31, 2020 12:02
@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
ehowey pushed a commit to ehowey/gatsby that referenced this pull request Jan 31, 2020
…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
})
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.

6 participants