feat: auto retry on deployment-related 499 errors #265

duyhungtnn · 2025-10-16T09:38:50Z

This pull request introduces a robust, reusable retry utility for asynchronous operations, applies it to critical networking and scheduling logic, and adds comprehensive tests to ensure reliability. The main improvements are the addition of a generic promiseRetriable function, refactoring of network and evaluation scheduling code to use this utility, and enhanced test coverage for both the new utility and its integrations.

Core Retry Utility:

Added a generic promiseRetriable function in PromiseRetriable.ts that provides configurable retry logic with support for linear and constant backoff strategies, and a customizable retry predicate. This includes the RetryPolicy and ShouldRetryFn types.

Networking Improvements:

Refactored postInternal in post.ts to use promiseRetriable, enabling automatic retries (up to 3 times) on deployment-related 499 errors (ClientClosedRequestException). The core POST logic was moved to a private _postInternal function. [1] [2]

Scheduler Improvements:

Refactored EvaluationTask to use promiseRetriable for fetching evaluations, simplifying retry logic and ensuring retries only occur while the task is running. Removed manual retry counting and rescheduling logic. [1] [2] [3]

Testing Enhancements:

Added comprehensive tests for promiseRetriable covering success, failure, delay, backoff strategies, and early termination scenarios in PromiseRetriable.spec.ts.
Added tests for postInternal to verify retry behavior on 499 errors in post.spec.ts.
Added a test for ApiClientImpl to ensure it calls postInternal correctly in ApiClientImpl.spec.ts.
Improved cleanup and reliability in EvaluationTask.spec.ts by ensuring task.stop() is always called and mocks are restored. [1] [2]

Error Handling:

Updated error handling in BKTClientImpl to ignore errors from the storage layer when logging evaluation failures, preventing unhandled promise rejections. [1] [2]

Refactored fetchEvaluationsInternal to accept options for shouldTrackFailure and timeoutMillis, enabling failure tracking only on the last allowed retry. Updated EvaluationTask to set shouldTrackFailure accordingly and added tests to verify correct behavior for failure tracking during retries.

Removed '(merged)' from the test suite name to accurately reflect the test context in EvaluationTask.spec.ts.

Copilot

Pull Request Overview

This PR refactors evaluation fetching to support conditional failure tracking during retries and adds tests to verify that tracking only occurs on the last allowed retry.

Introduces an options object to fetchEvaluationsInternal to control shouldTrackFailure and timeoutMillis.
Updates EvaluationTask to flag only the last retry for failure tracking and exposes getRetryCount for assertions.
Adds unit tests for conditional tracking and improves test cleanup.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
test/internal/scheduler/EvaluationTask.spec.ts	Adds tests validating shouldTrackFailure behavior and tracking calls; adjusts suite name and test cleanup.
src/internal/scheduler/EvaluationTask.ts	Passes shouldTrackFailure based on last retry, adds isLastTry and getRetryCount.
src/BKTClient.ts	Refactors fetchEvaluations to pass options; fetchEvaluationsInternal now accepts options and conditionally tracks failures.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/BKTClient.ts

src/internal/scheduler/EvaluationTask.ts

Refactored the fetchEvaluationsInternal method in BKTClientImpl to handle default options more explicitly and avoid mutation. Added comments to clarify the purpose of getRetryCount in EvaluationTask.

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/BKTClient.ts

src/internal/scheduler/EvaluationTask.ts

Adjusted spacing in constructor and method calls for consistency. Set a default value for the options parameter in fetchEvaluationsInternal.

Added catch blocks to suppress errors from the storage layer when logging evaluation events, ensuring that these errors do not affect the main execution flow.

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/BKTClient.ts

test/internal/scheduler/EvaluationTask.spec.ts

Simplified the way options are passed and destructured in fetchEvaluationsInternal for clarity and conciseness. Also reset the task variable to undefined in EvaluationTask.spec.ts after cleanup.

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/BKTClient.ts

test/internal/scheduler/EvaluationTask.spec.ts

Simplified destructuring of the options parameter by removing the unnecessary nullish coalescing operator. This change improves code clarity.

Updated the mock implementation in EvaluationTask.spec.ts to use parameter types from BKTClientImpl.fetchEvaluationsInternal, improving type safety and maintainability in the test.

Introduces a futureRetriable function for retrying async operations with configurable retry policy and error filtering. Includes comprehensive unit tests covering success, delay, max retries, and conditional retry logic.

Renamed FutureRetriable.ts and its test to PromiseRetriable.ts, updating all references and function names accordingly. Refactored postInternal to use promiseRetriable for retry logic on deployment-related 499 errors, improving clarity and consistency in naming.

Introduces a 'backoffStrategy' option to the RetryPolicy, supporting 'linear' (default) and 'constant' delay strategies for retry attempts. Updates implementation to calculate delay per attempt and adds tests for both linear and constant backoff behaviors.

Introduces unit tests for ApiClientImpl to verify it calls postInternal with correct parameters, and for postInternal to ensure it retries on 499 Client Closed Request errors. These tests improve coverage and validate error handling and request logic.

Simplifies fetchEvaluationsInternal by removing the shouldTrackFailure option and always tracking failures. Refactors EvaluationTask to use a generic promiseRetriable utility for retries, removing custom retry state and logic. Updates tests to remove now-obsolete failure tracking and retry count tests.

Copilot

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/internal/remote/post.ts

src/internal/scheduler/EvaluationTask.ts

Added 'backoffStrategy' to the retry policy in postInternal and ensured promiseRetriable is only called when EvaluationTask is running. This improves control over retry behavior and prevents unnecessary retries when the task is not active.

Copilot

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/internal/scheduler/EvaluationTask.ts

src/internal/remote/post.ts

Added logic to EvaluationTask to prevent retrying if the polling interval is less than or equal to the retry interval. Updated tests to verify that retries are skipped when the polling interval is short enough.

Copilot

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/internal/remote/PromiseRetriable.ts

test/internal/remote/post.spec.ts

Replaces runAllTimersAsync with advanceTimersByTimeAsync(1000) to more precisely control timer advancement during retry delay in postInternal tests. Also adds a comment for future cancellation support in PromiseRetriable.

- Fix scheduler stopping when pollingInterval <= retryInterval on error - Fix scheduler stopping after successful fetch with retryCount === 0 - Add tests to verify continuous scheduling behavior

Extended the test to verify that the timer triggers multiple times by checking the request count after running pending timers twice.

Copilot

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/BKTClient.ts

src/internal/remote/PromiseRetriable.ts

Adds console.debug statements to log errors from the storage layer in fetchEvaluations trackFailure and trackSuccess handlers. Also fixes a comment formatting issue in PromiseRetriable.ts.

src/BKTClient.ts

Updated logging in fetchEvaluations to use console.error instead of console.debug for storage layer errors, making error visibility more prominent.

cre8ivejp

Thank you!

duyhungtnn requested a review from Copilot October 16, 2025 09:39

Fix suite name typo in EvaluationTask.spec.ts

412ff89

Removed '(merged)' from the test suite name to accurately reflect the test context in EvaluationTask.spec.ts.

Copilot AI reviewed Oct 16, 2025

View reviewed changes

src/BKTClient.ts Outdated Show resolved Hide resolved

src/BKTClient.ts Outdated Show resolved Hide resolved

src/internal/scheduler/EvaluationTask.ts Outdated Show resolved Hide resolved

Refactor options handling in fetchEvaluationsInternal

8c25903

Refactored the fetchEvaluationsInternal method in BKTClientImpl to handle default options more explicitly and avoid mutation. Added comments to clarify the purpose of getRetryCount in EvaluationTask.

duyhungtnn requested a review from Copilot October 16, 2025 09:55

Copilot AI reviewed Oct 16, 2025

View reviewed changes

src/BKTClient.ts Show resolved Hide resolved

src/BKTClient.ts Outdated Show resolved Hide resolved

src/BKTClient.ts Show resolved Hide resolved

src/internal/scheduler/EvaluationTask.ts Outdated Show resolved Hide resolved

duyhungtnn added 2 commits October 16, 2025 17:02

Refactor formatting in BKTClientImpl methods

05562df

Adjusted spacing in constructor and method calls for consistency. Set a default value for the options parameter in fetchEvaluationsInternal.

Ignore storage layer errors in evaluation logging

637f365

Added catch blocks to suppress errors from the storage layer when logging evaluation events, ensuring that these errors do not affect the main execution flow.

duyhungtnn requested a review from Copilot October 16, 2025 10:07

duyhungtnn self-assigned this Oct 16, 2025

Copilot AI reviewed Oct 16, 2025

View reviewed changes

src/BKTClient.ts Outdated Show resolved Hide resolved

src/BKTClient.ts Outdated Show resolved Hide resolved

test/internal/scheduler/EvaluationTask.spec.ts Outdated Show resolved Hide resolved

Refactor fetchEvaluations options handling

0690d13

Simplified the way options are passed and destructured in fetchEvaluationsInternal for clarity and conciseness. Also reset the task variable to undefined in EvaluationTask.spec.ts after cleanup.

duyhungtnn requested a review from Copilot October 16, 2025 10:14

Copilot AI reviewed Oct 16, 2025

View reviewed changes

src/BKTClient.ts Outdated Show resolved Hide resolved

test/internal/scheduler/EvaluationTask.spec.ts Outdated Show resolved Hide resolved

duyhungtnn added 2 commits October 16, 2025 17:19

Remove redundant nullish coalescing in options destructure

cf642f9

Simplified destructuring of the options parameter by removing the unnecessary nullish coalescing operator. This change improves code clarity.

Refactor test to use fetchEvaluationsInternal parameter types

cc7edb2

Updated the mock implementation in EvaluationTask.spec.ts to use parameter types from BKTClientImpl.fetchEvaluationsInternal, improving type safety and maintainability in the test.

duyhungtnn requested a review from cre8ivejp October 16, 2025 10:20

duyhungtnn added 2 commits October 16, 2025 21:49

feat: add generic retry utility with tests

846f491

Introduces a futureRetriable function for retrying async operations with configurable retry policy and error filtering. Includes comprehensive unit tests covering success, delay, max retries, and conditional retry logic.

duyhungtnn marked this pull request as draft October 16, 2025 15:44

duyhungtnn added 3 commits October 16, 2025 22:50

duyhungtnn requested a review from Copilot October 17, 2025 03:02

Copilot AI reviewed Oct 17, 2025

View reviewed changes

src/internal/remote/post.ts Show resolved Hide resolved

src/internal/scheduler/EvaluationTask.ts Outdated Show resolved Hide resolved

duyhungtnn changed the title ~~feat: add conditional failure tracking to evaluation fetch retries~~ feat: auto retry on deployment-related 499 errors Oct 17, 2025

Refine retry logic in post and EvaluationTask

69c03c9

Added 'backoffStrategy' to the retry policy in postInternal and ensured promiseRetriable is only called when EvaluationTask is running. This improves control over retry behavior and prevents unnecessary retries when the task is not active.

duyhungtnn requested a review from Copilot October 17, 2025 03:14

duyhungtnn marked this pull request as ready for review October 17, 2025 03:15

Copilot AI reviewed Oct 17, 2025

View reviewed changes

src/internal/scheduler/EvaluationTask.ts Outdated Show resolved Hide resolved

src/internal/remote/post.ts Show resolved Hide resolved

Skip retries when polling interval is short

5915cb8

Added logic to EvaluationTask to prevent retrying if the polling interval is less than or equal to the retry interval. Updated tests to verify that retries are skipped when the polling interval is short enough.

duyhungtnn requested a review from Copilot October 17, 2025 03:47

Copilot AI reviewed Oct 17, 2025

View reviewed changes

src/internal/remote/PromiseRetriable.ts Show resolved Hide resolved

test/internal/remote/post.spec.ts Outdated Show resolved Hide resolved

duyhungtnn added 5 commits October 17, 2025 11:25

Improve timer handling in postInternal test

7b18504

Replaces runAllTimersAsync with advanceTimersByTimeAsync(1000) to more precisely control timer advancement during retry delay in postInternal tests. Also adds a comment for future cancellation support in PromiseRetriable.

fix(scheduler): prevent EvaluationTask from stopping after fetch

71dcd62

- Fix scheduler stopping when pollingInterval <= retryInterval on error - Fix scheduler stopping after successful fetch with retryCount === 0 - Add tests to verify continuous scheduling behavior

Add test for repeated timer execution in EvaluationTask

a915fa9

Extended the test to verify that the timer triggers multiple times by checking the request count after running pending timers twice.

chore: remove redundant code

1c3b790

revert: restore code for EvaluationTask

e82cf2c

duyhungtnn requested a review from Copilot October 21, 2025 08:46

Copilot AI reviewed Oct 21, 2025

View reviewed changes

src/BKTClient.ts Outdated Show resolved Hide resolved

src/BKTClient.ts Outdated Show resolved Hide resolved

src/internal/remote/PromiseRetriable.ts Outdated Show resolved Hide resolved

Add debug logging for storage layer errors

e24753a

Adds console.debug statements to log errors from the storage layer in fetchEvaluations trackFailure and trackSuccess handlers. Also fixes a comment formatting issue in PromiseRetriable.ts.

cre8ivejp reviewed Oct 21, 2025

View reviewed changes

src/BKTClient.ts Outdated Show resolved Hide resolved

src/BKTClient.ts Outdated Show resolved Hide resolved

Change storage error logging from debug to error

a53ba5b

Updated logging in fetchEvaluations to use console.error instead of console.debug for storage layer errors, making error visibility more prominent.

cre8ivejp approved these changes Oct 22, 2025

View reviewed changes

cre8ivejp merged commit e924bff into main Oct 22, 2025
4 checks passed

cre8ivejp deleted the fix/ignore-error-log-when-in-retry-process branch October 22, 2025 03:38

bucketeer-bot mentioned this pull request Oct 21, 2025

chore: release v2.4.0 #256

Merged

duyhungtnn mentioned this pull request Nov 7, 2025

feat: auto retry on deployment-related 499 errors bucketeer-io/android-client-sdk#240

Open

feat: auto retry on deployment-related 499 errors #265

feat: auto retry on deployment-related 499 errors #265

Uh oh!

Conversation

duyhungtnn commented Oct 16, 2025 • edited by cre8ivejp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cre8ivejp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

duyhungtnn commented Oct 16, 2025 •

edited by cre8ivejp

Loading