feat(gax): differentiate between server side and client side errors by shivanee-p · Pull Request #9451 · googleapis/google-cloud-node

shivanee-p · 2026-09-25T03:34:11Z

Align error.type and span error attributes with OTel semantic conventions

Fixes:

Resolves OpenTelemetry error.type according to the standardized 5-tier precedence
Exception formatting for server-side errors
Parses plan object errors for valid status codes
Unit testing
E2E testing for traceCall() using spans and exception events

gemini-code-assist

Code Review

This pull request enhances the observability and tracing capabilities of google-gax by implementing a robust 5-tier hierarchy for resolving the OpenTelemetry error.type attribute, extracting and reporting server.address and server.port from settings and environment variables, and improving exception event recording to include server-side status details and metadata. The review feedback is highly constructive, pointing out critical compatibility issues such as potential runtime crashes in browser environments due to direct Buffer references, and unnecessary parsing calls caused by truthy empty arrays returned from gRPC metadata lookups. Additionally, the feedback offers valuable performance optimizations by suggesting the migration of local arrays to module-level Set constants to prevent redundant allocations, and recommends validating environment-provided port numbers for robustness.

gemini-code-assist · 2026-09-25T03:37:39Z

+  const serverPort = env.GOOGLE_SDK_NODE_SERVER_PORT || env.SERVER_PORT;
+  if (serverPort && !isNaN(Number(serverPort))) {
+    result.serverPort = Number(serverPort);
+  }


If serverPort is an empty string or whitespace, parsing it directly might result in unexpected behavior or NaN. We should safely parse it to a number and validate that it is a valid port range (1-65535) before assigning it.

Suggested change

const serverPort = env.GOOGLE_SDK_NODE_SERVER_PORT || env.SERVER_PORT;

if (serverPort && !isNaN(Number(serverPort))) {

result.serverPort = Number(serverPort);

}

const serverPort = env.GOOGLE_SDK_NODE_SERVER_PORT || env.SERVER_PORT;

const parsedPort = serverPort ? Number(serverPort) : NaN;

if (!isNaN(parsedPort) && parsedPort > 0 && parsedPort <= 65535) {

result.serverPort = parsedPort;

}

shivanee-p · 2026-09-25T04:26:07Z

/gemini review

gemini-code-assist

Code Review

This pull request enhances OpenTelemetry tracing in google-gax by implementing a 5-tier error type resolution hierarchy and recording server-side error details (such as status details and metadata) within the exception stack trace. It also adds support for extracting and recording server.address and server.port attributes from settings and environment variables. Feedback on the changes highlights potential classification issues where plain error objects might be misidentified as pre-connection failures or client-side errors due to overly strict type checks. Additionally, a performance optimization was suggested to check the length of the returned gRPC metadata array to avoid redundant parsing of status details.

shivanee-p · 2026-09-25T04:52:23Z

/gemini review

gemini-code-assist

Code Review

This pull request implements a 5-tier error type resolution hierarchy for OpenTelemetry tracing in google-gax, aligning with semantic conventions. It introduces robust client-side and server-side error classification, extracts and records server address and port attributes, and captures detailed server-side exception metadata (such as status details and GFE metadata) within the exception stack trace. The feedback highlights opportunities to improve robustness in the telemetry path, specifically by wrapping JSON.stringify calls in try-catch blocks to prevent crashes from non-serializable properties, refactoring the recursive isPreConnectionFailure function into a loop to avoid stack overflows, and ensuring Buffer.isBuffer is a function before invocation in non-Node environments.

gemini-code-assist · 2026-09-25T04:54:45Z

+function isBuffer(val: unknown): val is Buffer {
+  return typeof Buffer !== 'undefined' && Buffer.isBuffer(val);
+}


In non-Node.js environments (such as browsers or custom runtimes), a global Buffer object might exist but may not implement the standard isBuffer method. To prevent a potential runtime TypeError, verify that Buffer.isBuffer is a function before calling it.

function isBuffer(val: unknown): val is Buffer { return ( typeof Buffer !== 'undefined' && typeof Buffer.isBuffer === 'function' && Buffer.isBuffer(val) ); }

danieljbruce

Looks great! A few optional nits, but feel free to backlog them and merge. Thanks for making errors more clear on our clients.

danieljbruce · 2026-09-25T17:32:45Z

+    depth++;
+  }
+
+  return undefined;


nit: I think this is equivalent to just return.

…onventions - Set span.status.message for HTTP and gRPC failure paths - Implement 5-tier error.type resolution hierarchy unwrapping e.cause - Omit response status codes on client-side errors without server response - Conditionally set server.address and server.port, omitting on pre-connection failures - Record client stack traces on client errors and server error details in exception.stacktrace for server errors - Avoid recording metadata, status_details, domain, reason, or error_info_metadata as span/event attributes - Update harness and unit tests for all error scenarios

…util - Safely check typeof Buffer !== 'undefined' before accessing Buffer in TracerHelper - Move error codes, generic classes, and pre-connection codes from loops in TracerHelper to util.ts - Move ignoredClientHeaderTokens from loop in metadataResolver to util.ts - Add unit tests for Buffer handling and exported util constants

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

- Check resolveHttpStatusCode and resolveRpcStatusName before stack check in isPreConnectionFailure - Allow plain error objects carrying valid status codes in isServerSideError - Add unit tests verifying plain objects retain server address/port and resolve error.type

…cular errors in TracerHelper

…error loop

## [6.8.0](google-gax-v6.7.0...google-gax-v6.8.0) (2026-09-25) ### Features * **gax:** Differentiate between server side and client side errors ([#9451](#9451)) ([c477918](c477918))

gemini-code-assist Bot reviewed Sep 25, 2026

View reviewed changes

Comment thread core/packages/gax/src/observability/TracerHelper.ts Outdated

Comment thread core/packages/gax/src/observability/TracerHelper.ts Outdated

Comment thread core/packages/gax/src/observability/TracerHelper.ts

gemini-code-assist Bot reviewed Sep 25, 2026

View reviewed changes

shivanee-p force-pushed the shivaneep-errors-attributes branch from 1ca783f to 2387b3d Compare September 25, 2026 15:41

shivanee-p marked this pull request as ready for review September 25, 2026 15:41

shivanee-p requested a review from a team as a code owner September 25, 2026 15:41

github-actions Bot requested a review from danieljbruce September 25, 2026 15:41

danieljbruce approved these changes Sep 25, 2026

View reviewed changes

danieljbruce reviewed Sep 25, 2026

View reviewed changes

Comment thread core/packages/gax/src/observability/metadataResolver.ts

danieljbruce reviewed Sep 25, 2026

View reviewed changes

Comment thread core/packages/gax/src/observability/TracerHelper.ts Outdated

danieljbruce reviewed Sep 25, 2026

View reviewed changes

shivanee-p and others added 7 commits September 25, 2026 10:45

Update core/packages/gax/src/observability/TracerHelper.ts

ac144c1

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update core/packages/gax/src/observability/TracerHelper.ts

8b2caeb

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

fix(gax): safely serialize status details and metadata and handle cir…

0918ae8

…cular errors in TracerHelper

refactor(gax): document helper functions and refactor client network …

d2e2888

…error loop

shivanee-p force-pushed the shivaneep-errors-attributes branch from 2387b3d to d2e2888 Compare September 25, 2026 17:46

refactor(gax): use DEPTH_TO_CHECK constant for error traversal loops

46c83f2

shivanee-p merged commit c477918 into main Sep 25, 2026
50 checks passed

shivanee-p deleted the shivaneep-errors-attributes branch September 25, 2026 18:04

shivanee-p mentioned this pull request Sep 25, 2026

chore: release google-gax 6.8.0 #9453

Merged

Conversation

shivanee-p commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes:

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

shivanee-p commented Sep 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shivanee-p commented Sep 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

danieljbruce left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

danieljbruce Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shivanee-p commented Sep 25, 2026 •

edited

Loading

danieljbruce left a comment •

edited

Loading