GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — fix(start-proxy): explicitly fail when proxy retries are exhausted by iaadillatif · Pull Request #3531 · github/codeql-action Conversation
|
| File | Description |
|---|---|
| src/start-proxy/launcher.ts | New launcher implementing retries, stabilization delay checks, and explicit failure on exhaustion. |
| src/start-proxy/launcher.test.ts | New tests covering retry exhaustion, spawn error propagation, and success path. |
| src/start-proxy-action.ts | Uses the extracted launcher instead of inline startup logic. |
| lib/start-proxy-action.js | Generated build output reflecting the TypeScript changes (not reviewed). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| test("startProxy throws explicit error when retries are exhausted", async (t) => { | ||
| const fakeSpawn = sinon.stub().callsFake(() => { | ||
| const process = new FakeChildProcess(); | ||
| // Emit exit on next event loop iteration, ensuring it fires during the delay | ||
| setImmediate(() => { |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This test will wait on the real util.delay(1000) inside startProxy (and retry scenarios will hit it repeatedly), which will slow down the test suite and can make timing-dependent tests flakier. Consider stubbing util.delay (or injecting a delay function into startProxy) so the tests run quickly and deterministically.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| subprocess.unref(); | ||
| if (subprocess.pid) { | ||
| core.saveState("proxy-process-pid", `${subprocess.pid}`); | ||
| } |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
core.saveState("proxy-process-pid", ...) is called immediately after spawn(), before the stabilization delay. If the child exits during the delay (or on a later retry), the post-action step (start-proxy-action-post.ts) will still read this PID and call process.kill(pid), which can end up targeting an unrelated process if the PID is reused. Consider only saving the PID after the proxy has remained alive past the startup delay (i.e., right before returning success), or otherwise persisting a PID only for the final, stabilized process.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
At least 1 approving review is required to merge this pull request.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
[BUG]: Proxy startup may report success when retries are exhausted and process does not stabilize
There was an error while loading. Please reload this page.