Skip to content

fix: fail on bootstrap download error #59

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

Merged
merged 1 commit into from
Apr 3, 2025

Conversation

mcdurdin
Copy link
Member

This addresses a situation where a network glitch can cause a single file in the bootstrap to fail to download, but the bootstrap itself would continue. This tended to lead to strange site issues with missing files, which can be hard to trace.

Fixes: #58

This addresses a situation where a network glitch can cause a single
file in the bootstrap to fail to download, but the bootstrap itself
would continue. This tended to lead to strange site issues with missing
files, which can be hard to trace.

Fixes: #58
@mcdurdin mcdurdin added this to the B18S4 milestone Mar 20, 2025
@mcdurdin mcdurdin added the fix label Mar 20, 2025
@mcdurdin mcdurdin requested a review from darcywong00 March 20, 2025 05:29
Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -58,7 +58,7 @@ function _bootstrap_download() {
curl -H "Cache-Control: no-cache" -fs "https://raw.githubusercontent.com/keymanapp/shared-sites/$BOOTSTRAP_VERSION/$remote_file" -o "$local_file" || (
_bootstrap_echo "FATAL: Failed to download $remote_file"
exit 3
)
) || exit $?
Copy link
Contributor

Choose a reason for hiding this comment

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

So is exit 3 not doing anything?

Copy link
Member Author

Choose a reason for hiding this comment

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

It exits the subshell (...) but because it's in a pipe it doesn't fail the entire command. I didn't want to use set -o pipefail because of other possible side-effects

@darcywong00 darcywong00 modified the milestones: B18S4, B18S5 Mar 29, 2025
Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

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

lgtm

@mcdurdin mcdurdin merged commit a9da5ac into main Apr 3, 2025
2 checks passed
@mcdurdin mcdurdin deleted the fix/58-fail-on-bootstrap-download-error branch April 3, 2025 05:55
@github-project-automation github-project-automation bot moved this to Done in Keyman Apr 3, 2025
@mcdurdin
Copy link
Member Author

mcdurdin commented Apr 3, 2025

Note: given this change impacts only deployment, I am not preparing a new release for this PR. We can do a new release when we have other changes to deploy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

bug: bootstrap 'fatal' download error doesn't die
2 participants