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

feat(installer): retry download if connection is interrupted #5126

Conversation

aslushnikov
Copy link
Collaborator

Fixes #5110

@yury-s
Copy link
Member

yury-s commented Jan 23, 2021

Why is it specific to ECONRESET?

@@ -173,7 +173,18 @@ export async function downloadBrowserWithProgressBar(browsersPath: string, brows
const url = revisionURL(browser);
const zipPath = path.join(os.tmpdir(), `playwright-download-${browser.name}-${browserPaths.hostPlatform}-${browser.revision}.zip`);
try {
await downloadFile(url, zipPath, progress);
for (let attempt = 1, N = 3; attempt <= N; ++attempt) {
const {error} = await downloadFile(url, zipPath, progress);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not try/catch instead? Are you sure we are not getting errors anywhere else? Like, from fs.createWriteStream for example?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there's a try-catch around to handle this case. We don't want to re-try in case of write stream error though since it's hard to predict consequences.

@aslushnikov aslushnikov force-pushed the retry-browser-download-if-network-is-intermittent branch from 0d33efb to 68fb8f0 Compare January 28, 2021 08:19
@aslushnikov
Copy link
Collaborator Author

Why is it specific to ECONRESET?

@yury-s this is the only error we see so far that's worth a retry. Dmitry also mentioned ETIMEDOUT (e.g. here), but I think this happens since our jobs are often interrupted on Mac 11 (there's mac11 shortage on gh)

@aslushnikov aslushnikov merged commit fe1302b into microsoft:master Jan 28, 2021
@aslushnikov aslushnikov deleted the retry-browser-download-if-network-is-intermittent branch January 28, 2021 08:37
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.

[BUG] Lock not released in case of ECONNRESET when installing
3 participants