-
Notifications
You must be signed in to change notification settings - Fork 76
Handle failure from grep/awk #256
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #255 by preventing script failure when grep doesn't find a match in the checksum file. The change adds || true to the grep/awk pipeline so that an empty result is handled gracefully by the existing error checking logic.
Key Changes:
- Added
|| trueto the grep/awk command extracting the expected checksum to prevent pipeline failure when no match is found
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
afinetooth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsoref Thank you for taking the time to both file this issue report and submit a proposed fix!
After reviewing the issue, your PR, and the full CI build log from the GitHub Actions failure, we’ve identified the root cause of this problem.
The failure occurred because the workflow attempted to download coveralls-checksums.txt from the GitHub Releases page, but instead of retrieving the expected plain-text checksums file, it was served an HTML error page. The HTML content indicated a "No server is currently available to service your request" message, meaning this was a transient server-side issue on GitHub’s infrastructure (likely a 503 Service Unavailable error). This resulted in the script attempting to parse an invalid file, ultimately leading to the observed failure.
The current code already handles grep returning no matches gracefully—it leaves expected_checksum as empty, which is then caught by the [ -z "$expected_checksum" ] condition. This triggers an appropriate error message and exits the workflow, adhering to the script's intended behavior. Adding || true to suppress grep errors would weaken this security-critical checksum comparison step by allowing the script to proceed without successfully verifying the file, which is not desirable.
The real issue here was not with the current script but with the transient server-side failure on GitHub’s artifact/download infrastructure. To address this kind of situation more robustly, we might look into additional retries or better error handling upstream where the file is downloaded, rather than suppressing errors during the checksum validation phase.
If you have further insights into why the server might have returned the error page or believe there’s another aspect to consider, please let us know. Additionally, if there are other improvements or ideas you’d like to discuss, we’d welcome your input.
|
If the 503s from github happen often enough, especially at the scale that your code runs. The |
grepfailure handling is lousy #255