VPAAMP-698: Improve retry logic for CURL errors in AAMP#1690
Open
psiva01 wants to merge 6 commits into
Open
Conversation
Reason for change: Added centralized helper for CURL error handling and included all the transient curl failures typically treated as retryable. Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Improves AAMP download retry behavior by expanding the set of CURL transport errors treated as retryable and adding helper logic to make the retry decision clearer and more consistent across download paths.
Changes:
- Added
IsRetryableCurlFailure()helper(s) to classify transient CURL transport failures as retryable. - Updated retry decision logic in
PrivateInstanceAAMP::GetFile()andAampCurlDownloader::Download()to use the new helper. - Added parameterized functional coverage for newly retryable CURL failures in the
AampCurlDownloaderutests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/utests/tests/AampCurlDownloader/FunctionalTests.cpp | Adds a parameterized test verifying retries for additional retryable CURL failures. |
| priv_aamp.cpp | Introduces retryable CURL failure classification helper and refactors retry condition in GetFile(). |
| downloader/AampCurlDownloader.cpp | Adds retryable CURL failure classification helper and uses it in the curl-failure retry condition. |
Contributor
Author
|
@copilot implement the review comment from #1690 (comment). |
Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>
Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new utility function to centralize the handling of retryable curl transport failures and updates the retry logic across the downloader codebase to use this new function. It also adds comprehensive unit tests to ensure that all newly classified retryable curl errors are correctly retried.
Retry Logic Improvements:
IsRetryableCurlFailure(CURLcode curlCode)toAampUtils.cppand its header, which encapsulates logic for determining if a curl error is transient and should be retried. [1] [2]AampCurlDownloader::DownloadandPrivateInstanceAAMP::GetFileto useIsRetryableCurlFailureinstead of ad-hoc error checks, ensuring consistency and easier future maintenance. [1] [2]Testing Enhancements:
RetryableCurlFailureTestsinFunctionalTests.cppto verify that all errors classified as retryable byIsRetryableCurlFailureare actually retried and handled as expected.FakeAampUtils.cppto provide a stub forIsRetryableCurlFailure, supporting test coverage.These changes improve the robustness and maintainability of curl error handling and make the retry mechanism more transparent and testable.