-
Notifications
You must be signed in to change notification settings - Fork 673
fix: add handling for 404 errors in scale-down tests and improve error logging #4726
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
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
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 improves 404 error handling in the scale-down logic for GitHub self-hosted runners. When checking runner state, 404 responses are now treated as valid indicators that a runner no longer exists on GitHub, rather than as errors that block further processing.
- Returns
null
fromgetGitHubSelfHostedRunnerState
when a 404 error occurs, allowing downstream logic to handle missing runners appropriately - Updates dependent functions to handle the new nullable return type
- Adds comprehensive test coverage for 404 error scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
scale-down.ts | Adds 404 error handling to treat missing runners as valid state, updates function signatures and logic |
scale-down.test.ts | Adds test cases for 404 error handling scenarios including orphan detection and busy state checking |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lambdas/functions/control-plane/src/scale-runners/scale-down.test.ts
Outdated
Show resolved
Hide resolved
…est.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
When looking at the API spec, it looks like 404 is not even a valid return code. Get a self-hosted runner for an organization. I have reached out to Github to ask if this a documentation problem or if its a API problem. |
@npalm any update on this? |
Github have come back and confirmed that this is correct behavior and is documented https://docs.github.com/en/enterprise-cloud@latest/rest/using-the-rest-api/troubleshooting-the-rest-api?apiVersion=2022-11-28#404-not-found-for-an-existing-resource |
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.
@stuartp44 thx, tested and looks good!
🤖 I have created a release *beep* *boop* --- ## [6.7.4](v6.7.3...v6.7.4) (2025-08-25) ### Bug Fixes * add handling for 404 errors in scale-down tests and improve error logging ([#4726](#4726)) ([95aa6a2](95aa6a2)) @stuartp44 * **lambda:** bump the aws group in /lambdas with 7 updates ([#4709](#4709)) ([0e74b3d](0e74b3d)) * **lambda:** bump the aws-powertools group in /lambdas with 4 updates ([#4718](#4718)) ([9a63469](9a63469)) * remove progress and control codes from log output ([#4699](#4699)) ([1c6b424](1c6b424)) @aarongorka --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: runners-releaser[bot] <194412594+runners-releaser[bot]@users.noreply.github.com>
This PR helps with dealing with 404 issues when terminating runners and checking if the runner is actually there or not. At the moment, we don't recognize the fact 404 is actually a good response for runners that are no longer there. This means the rest of the logic is ignored to do last chance checking. This PR helps by returning null for a 404, hence able to act on this or the runner data if we get it. For all other logic, we still return the previous error.