fix: possible tight busy loop on certain connection errors

[toddbaert]

With #1590, we fixed a few issues; one was a possible busy loop on certain connection errors. Unfortunately we missed the case that the error actually occurs on the stream (onError). This applies the same backoff in that case, and adds an associated test.

I tested this fix manually as well, with a Spring app and a flagd hard-coded to return errors.

Thanks @leakonvalinka and @guidobrei for finding this.

[gemini-code-assist]

Summary of Changes

Hello @toddbaert, 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 addresses a critical issue in the flagd provider where certain connection errors could lead to a tight busy loop, consuming excessive resources. It introduces a robust backoff mechanism, ensuring that stream reconnections are throttled appropriately after both synchronous and asynchronous error events, thereby improving the stability and resilience of the flag synchronization process.

Highlights

• Busy Loop Prevention: Implemented a backoff mechanism to prevent tight busy loops when connection errors occur, specifically addressing cases where errors are reported via the onError stream callback.
• Throttling Mechanism: Introduced an AtomicBoolean flag, shouldThrottle, to manage and apply exponential backoff before retrying stream connections after an error.
• Enhanced Error Handling: Modified SyncStreamObserver to propagate error states back to the main stream loop, ensuring that the backoff logic is triggered for all types of stream-related exceptions, including those from getMetadata and general syncStream issues.
• Improved Shutdown Logic: Updated the shutdown method to use AtomicBoolean.compareAndSet for a more robust and thread-safe single-execution guarantee.
• Comprehensive Testing: Added new unit tests (syncInitError_DoesNotBusyWait and asyncInitError_DoesNotBusyWait) to specifically validate the backoff behavior for both immediate (synchronous) and delayed (asynchronous) stream errors.

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 successfully addresses a potential busy loop on stream connection errors by introducing a backoff mechanism in the onError handler. The changes are well-implemented, centralizing the backoff logic for better maintainability. The addition of a new test case for asynchronous errors and the restructuring of existing tests ensure the fix is robust and well-covered. I have one suggestion to reduce code duplication in the test file, which will improve its maintainability.

[toddbaert]

Git is rendering this oddly; this is the previous unchanged test (other than the name). The new one is below.

[toddbaert]

This test exercises the fix (the onError runs in the asyncErrorStub).