fix: Transform network failures into specific TLS timeout

[thiyaguk09]

Description

This change refines error handling in util.makeRequest to intercept and transform common low-level network failures (ECONNRESET, ETIMEDOUT, "timed out", "TLS handshake") into three distinct and specific standard Error messages.

The following raw errors are now intercepted:

• ECONNRESET
• ETIMEDOUT
• Generic messages containing "timed out"
• Generic messages containing "TLS handshake"

Instead of a single message, developers now receive tailored diagnostic information: a specific message for TLS/CPU starvation issues, one for network timeouts, and one for connection resets. This provides clearer guidance for debugging. The original stack trace is preserved on the new Error object. This is an additive change with no breaking impacts, validated by a consolidated, data-driven unit test suite.

By transforming these errors and preserving the original stack trace, we prevent the propagation of cryptic, low-level network codes and provide developers with a clear, unified diagnostic message tailored to the type of connection failure experienced.

Impact

The impact of this change is primarily positive, improving the developer experience:

• Improved Error Diagnostics (Granular): Developers receive three distinct, specific messages instead of a single ambiguous one. For instance, a TLS handshake error receives a special message related to CPU starvation to guide performance debugging.
• Consistent Error Handling: Facilitates easier integration with custom error retry and logging mechanisms by providing a predictable, standard Error structure (augmented with the original stack trace) rather than a raw, non-standard network error object.
• No Breaking Changes: This is a purely additive fix that catches and transforms errors that would have been thrown anyway. It does not alter the successful path for requests.

Testing

Yes, unit tests were added.

• A dedicated unit test suite, Network Connectivity Errors, was created under makeAuthenticatedRequestFactory to validate the new transformation logic.

• The test structure was consolidated into a single, data-driven forEach loop that verifies the mapping of the four common failure modes to the three distinct output messages defined in the UtilExceptionMessages enum.

• A single test loop replaced four separate tests, covering all conditions:

  • should transform raw ECONNRESET into specific network error
  • should transform raw "TLS handshake" into specific network error
  • should transform raw generic "timed out" into specific network error
  • should transform raw ETIMEDOUT into specific network error

Tests Changed? No existing tests were modified.

Breaking Changes? No breaking changes are necessary.

Additional Information

• Error Object Change: The transformation logic was simplified to augment a standard JavaScript Error object.
• Test Structure: The tests were consolidated into a single forEach loop for improved clarity and maintainability.
• Stubbing: The authClient was stubbed to guarantee successful authorization, forcing execution into the network path where the error injection and transformation occur, preventing test timeouts.
  The logic ensures that if an error is transformed, the original stack trace (err.stack) is preserved on the new Error object, allowing developers to debug the source of the failure effectively.

Checklist

• Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease
• Appropriate docs were updated
• Appropriate comments were added, particularly in complex areas or places that require background
• No new warnings or issues will be generated from this change

Fixes #

[gemini-code-assist]

Summary of Changes

Hello @thiyaguk09, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant improvement to error handling within the util.makeRequest function. It standardizes the reporting of common low-level network and transport-layer failures, such as connection resets and timeouts, by transforming them into a consistent ApiError. This change aims to enhance developer experience by providing clear, actionable diagnostic messages, particularly for TLS handshake timeouts potentially caused by CPU starvation, and ensures more predictable error objects for easier integration with retry and logging mechanisms.

Highlights

• Standardized Error Handling: Implemented new error handling in util.makeRequest to intercept low-level network failures.
• Specific Error Transformation: Transforms ECONNRESET, ETIMEDOUT, and generic "timed out" or "TLS handshake" messages into a consistent ApiError.
• Actionable ApiError: The transformed error has code: 408 and a message: "TLS handshake timeout. This may be due to CPU starvation."
• Improved Diagnostics: Provides clearer, unified diagnostic messages, preventing propagation of ambiguous network errors.
• Comprehensive Testing: Added a new unit test suite with four dedicated tests to validate each specific error transformation condition.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces error handling to transform specific network errors into a 408 TLS handshake timeout ApiError. The implementation in util.makeRequest is functionally correct, but I've suggested a minor refactoring to improve maintainability by using constants. The new tests, however, have a critical flaw: they stub the very function they are supposed to test and re-implement its logic, meaning the production code is not actually being tested. I've provided a single, comprehensive comment on the test file that explains this issue and also addresses code duplication and a confusing test setup, along with a suggested refactoring that fixes all these points.

[mahendra-google]

LGTM

[thiyaguk09]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces error handling logic to transform network failures into specific TLS timeout ApiErrors. The changes include modifications to util.ts to intercept and transform raw errors like ECONNRESET, ETIMEDOUT, and generic timeout messages into a consistent ApiError. Additionally, new unit tests were added in util.ts to validate the new transformation logic. The review focuses on correctness and maintainability, with suggestions for code improvements and clarifications.

[thiyaguk09]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces improved error handling for network and transport-layer failures in util.makeRequest, transforming them into consistent and actionable ApiError instances. This enhancement prevents the propagation of ambiguous, underlying network library errors and provides developers with clear, unified diagnostic messages. The changes include intercepting ECONNRESET, ETIMEDOUT, and generic timeout/TLS handshake errors, and converting them into ApiError with codes 408 or 503. Unit tests have been added to validate the new transformation logic.

[thiyaguk09]

/gemini summary

[gemini-code-assist]

Summary of Changes

This pull request significantly improves the error handling within the util.makeRequest function by intercepting cryptic low-level network failures and transforming them into clear, actionable diagnostic messages. This change aims to provide developers with more precise information for debugging network and TLS-related issues, ultimately enhancing the developer experience without introducing any breaking changes.

Highlights

• Enhanced Error Transformation: Implemented a mechanism to intercept and transform common low-level network errors (ECONNRESET, ETIMEDOUT, "timed out", "TLS handshake") into more specific, user-friendly messages.
• Granular Error Messages: Introduced a new UtilExceptionMessages enum to provide three distinct error messages for TLS/CPU starvation, network timeouts, and connection resets, improving diagnostic clarity.
• Stack Trace Preservation: Ensures the original error's stack trace is preserved on the newly created error object, which is crucial for effective debugging.
• Comprehensive Unit Testing: Added a new, data-driven unit test suite that validates the transformation logic for all four intercepted error types, ensuring correct mapping to the three distinct output messages.

Changelog

• src/nodejs-common/util.ts
  • Defined UtilExceptionMessages enum to standardize specific error messages for TLS timeouts, ETIMEDOUT, and ECONNRESET.
  • Modified the makeRequest callback to check for specific error message patterns (tls handshake, etimedout, timed out, econnreset) in incoming errors.
  • If a match is found, a new Error object is created using the corresponding message from UtilExceptionMessages, and the original error's stack trace is preserved.
• test/nodejs-common/util.ts
  • Imported the new UtilExceptionMessages enum.
  • Added a new test suite named "Handling of TLS Handshake, Timeout, and Connection Reset Errors in Authenticated Requests".
  • Implemented a data-driven forEach loop to test various network error scenarios (ECONNRESET, "TLS handshake", generic "timed out", ETIMEDOUT) and assert that they are transformed into the expected UtilExceptionMessages.
  • Configured retryRequestOverride to simulate these network errors during testing.

Activity

• The gemini-code-assist[bot] provided an initial summary and critical feedback on the test strategy, suggesting mocking retry-request and refactoring tests into a data-driven approach.
• The author, thiyaguk09, initially dismissed the critical feedback but later implemented the suggested changes, confirming the removal of duplicate code and fixing the error object modification.
• Discussions occurred regarding the use of constants for error messages and the creation of new error objects versus modifying existing ones, leading to the current implementation using an enum and new Error objects.