Skip to content

feat(gax): add resolveExceptionStacktrace helper for exception.stacktrace - #9744

Merged
shivanee-p merged 6 commits into
mainfrom
shivaneep-o11y-stacktrace
Oct 30, 2026
Merged

shivanee-p merged 6 commits into
mainfrom
shivaneep-o11y-stacktrace

Conversation

@shivanee-p

@shivanee-p shivanee-p commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

This PR adds a resolveExceptionStacktrace helper so LoggerHelper can populate the exception.stacktrace log attribute from the JavaScript call stack, simplifies resolveServerExceptionDetails, and fixes timeout: 0 and shared backoffSettings handling during retrying calls.

Key Changes

  • Add resolveExceptionStacktrace(e: unknown) in errorUtil.ts to resolve exception.stacktrace from the first non-empty string stack on the error
  • Simplify resolveServerExceptionDetails to resolve only {message} while traversing the full .cause chain (up to DEPTH_TO_CHECK with cycle detection), removing synthetic statusDetails/metadata stacktrace formatting and eager gRPC status details decoding (raw payloads belong in L4 response.payload, not exception.stacktrace).
  • Clone retry copy-on-write via cloneRetryOptions in CallSettings.merge when per-call timeout or maxRetries overrides modify backoffSettings, preventing mutation of shared method defaults or caller-owned backoffSettings while keeping the 30s default fallback on CallSettings.timeout for 0/undefined.
  • Fix retryable in retries.ts to treat totalTimeoutMillis: 0 as an explicit deadline instead of ignoring it (Infinity), allowing attempt 1 to run once with a 0ms deadline (recording its attempt span) before stopping cleanly without spinning in a retry loop.
  • Add unit tests in errorUtil.ts, gax.ts, and apiCallable.ts covering resolveExceptionStacktrace, deep/circular .cause chains, CallSettings.merge copy-on-write isolation, and timeout: 0 retry behavior.

@shivanee-p
shivanee-p added this pull request to stack #9746 October 27, 2026 18:13

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the resolveExceptionStacktrace utility function to extract stack traces for failed calls, handling server-side and client-side errors differently, along with comprehensive unit tests. Feedback suggests adding defensive checks to ensure the error argument is a non-null object, preventing potential runtime crashes if a non-object is passed to the utility.

@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-stacktrace branch from 6f683da to 3dcc5dd Compare October 27, 2026 18:24
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-stacktrace branch from 3dcc5dd to 4011d85 Compare October 27, 2026 18:52
@shivanee-p
shivanee-p marked this pull request as ready for review October 27, 2026 23:25
@shivanee-p
shivanee-p requested a review from a team as a code owner October 27, 2026 23:25
@github-actions
github-actions Bot requested a review from feywind October 27, 2026 23:25
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-stacktrace branch 2 times, most recently from 1dd4794 to 3e15f55 Compare October 28, 2026 16:03

@quirogas quirogas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed #9744 against the Milestone 2 Error Logging design doc, the cross-language error logging specification, and live gRPC/REST (fallback: 'rest') error payloads. Left 1 blocking comment on resolveExceptionStacktrace (and its corresponding test case in test/unit/errorUtil.ts).

@quirogas quirogas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving to unblock merging once the inline comments above are addressed.

@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-stacktrace branch 3 times, most recently from 8a006fa to 18696e0 Compare October 29, 2026 20:35
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-stacktrace branch from 87d17ca to dfd3ac8 Compare October 30, 2026 02:12
Base automatically changed from shivaneep-o11y-error-domain to main October 30, 2026 02:52
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-stacktrace branch from dfd3ac8 to 15c8692 Compare October 30, 2026 02:52
…race

Add resolveExceptionStacktrace to errorUtil so LoggerHelper can
populate exception.stacktrace. Server-side errors use the status
details and response metadata formatted by resolveServerExceptionDetails,
and have no stacktrace when the server sent neither. All other errors
use the JavaScript stack (e.stack).
resolveExceptionStacktrace now takes unknown, matching
resolveExceptionMessage. Server-side Error instances still use the
server-provided details; all other values use their stack when it is a
string, so thrown strings, null, and plain objects have no stacktrace.
Check only the routing to resolveServerExceptionDetails instead of repeating its formatting test, cover the empty-stack check, and drop a non-Error case that took the same path as the string case.
…erse cause chain in resolveServerExceptionDetails
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-stacktrace branch from 15c8692 to 87abd2a Compare October 30, 2026 04:21
@shivanee-p
shivanee-p merged commit baed8c4 into main Oct 30, 2026
51 checks passed
@shivanee-p
shivanee-p deleted the shivaneep-o11y-stacktrace branch October 30, 2026 20:56
@release-please release-please Bot mentioned this pull request Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants