fix(k8s): prevent circular-JSON crash in catch-block debug logs#364
Open
remidebette wants to merge 1 commit into
Open
fix(k8s): prevent circular-JSON crash in catch-block debug logs#364remidebette wants to merge 1 commit into
remidebette wants to merge 1 commit into
Conversation
Six catch blocks in the k8s hooks built debug strings with
`JSON.stringify(err)` inside a template literal:
} catch (err) {
core.debug(`execPodStep failed: ${JSON.stringify(err)}`)
const message = (err as any)?.response?.body?.message || err
throw new Error(`failed to run script step: ${message}`)
}
When err is a @kubernetes/client-node HTTP error, its response embeds
a TLSSocket <-> HTTPParser cycle. JSON.stringify throws synchronously
before core.debug runs, propagates out of the catch block, and shadows
the original failure with:
TypeError: Converting circular structure to JSON
--> starting at object with constructor 'TLSSocket'
Add a defensive formatError() helper that prefers
response.body.message (the diagnostic K8s API surface), then
Error.stack/message, then a String() / safe-JSON fallback for
plain objects, never throwing. Use it at the six unsafe sites.
Closes actions#329.
Complements actions#341, which fixes a related but distinct pattern
(`throw new Error(... JSON.stringify(error) ...)` producing `{}`
because Error.message is non-enumerable). The two PRs touch
disjoint lines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Six catch blocks across the k8s hooks built debug strings with
JSON.stringify(err)inside a template literal. Whenerris a@kubernetes/client-nodeHTTP error (response embeds aTLSSocket↔HTTPParsercycle),JSON.stringifythrows synchronously beforecore.debugruns, propagating out of the catch and shadowing the original failure withTypeError: Converting circular structure to JSON.Adds a
formatError()helper that prefersresponse.body.message, thenError.stack/message, then a safe JSON /String()fallback — never throws.Linked issues
Relationship to #341
Complementary — #341 fixes a distinct pattern (
throw new Error(\… ${JSON.stringify(error)}`)producing{}becauseError.message` is non-enumerable). The two PRs touch disjoint lines.Tests
New
format-error-test.tswith 12 cases including a circular-ref repro mimicking theTLSSocket↔HTTPParsershape. All unit suites pass (66/66);lint,format-check,tsc + nccclean.