VPAAMP-698: Improve retry logic for CURL errors in AAMP

AampUtils.cpp

@@ -1546,3 +1546,27 @@ bool IsCurlTimeoutFailure( int httpResponseCode )
 			return false;
 	}
 }
+
+/**
+ * @brief Check if a curl failure is retryable based on transient transport errors
+ * @param[in] curlCode curl error code
+ * @retval true if the error is retryable
+ * @retval false otherwise
+ */
+bool IsRetryableCurlFailure( CURLcode curlCode )
+{
+	switch( curlCode )
+	{
+		case CURLE_OPERATION_TIMEDOUT:
+		case CURLE_PARTIAL_FILE:
+		case CURLE_COULDNT_RESOLVE_PROXY:
+		case CURLE_COULDNT_RESOLVE_HOST:
+		case CURLE_COULDNT_CONNECT:
+		case CURLE_RECV_ERROR:
+		case CURLE_SEND_ERROR:
+		case CURLE_GOT_NOTHING:
+			return true;
+		default:
+			return false;
+	}
+}

AampUtils.h

@@ -469,4 +469,6 @@ CurlTimeoutFailureReason GetCurlTimeoutFailureReason(CURL* curl);
 
 bool IsCurlTimeoutFailure( int httpResponseCode );
 
+bool IsRetryableCurlFailure( CURLcode curlCode );
+
 #endif  /* __AAMP_UTILS_H__ */

downloader/AampCurlDownloader.cpp

@@ -115,7 +115,6 @@ long aamp_CurlEasyGetinfoLong( CURL *handle, CURLINFO info )
 	}
 	return rc;
 }
-
 char *aamp_CurlEasyGetinfoString( CURL *handle, CURLINFO info )
 {
 	char *rc = NULL;
@@ -368,7 +367,8 @@ int AampCurlDownloader::Download(const std::string &urlStr, std::shared_ptr<Down
 				{
 					if(numDownloadAttempts <= numRetriesAllowed)
 					{ //Attempt retry for partial downloads, which have a higher chance to succeed
-						if (httpRetVal == CURLE_COULDNT_CONNECT || IsCurlTimeoutFailure (httpRetVal) || httpRetVal == CURLE_SEND_ERROR)
+						if (IsRetryableCurlFailure(static_cast<CURLcode>(httpRetVal)) ||
+							IsCurlTimeoutFailure(httpRetVal))
 						{
 							AAMPLOG_WARN("Download failed due to curl error %d numDownloadAttempts %d numRetriesAllowed %d", httpRetVal, numDownloadAttempts, numRetriesAllowed);
 							loopAgain = true;

priv_aamp.cpp

@@ -4990,10 +4990,11 @@ bool PrivateInstanceAAMP::GetFile( std::string remoteUrl, AampMediaType mediaTyp
 						print_headerResponse(context.allResponseHeaders, mediaType);
 
 					}
-						if (res == CURLE_COULDNT_CONNECT || IsCurlTimeoutFailure(res) ||
+						const bool isRetryableStall =
 							(isDownloadStalled &&
-								(eCURL_ABORT_REASON_LOW_BANDWIDTH_TIMEDOUT != abortReason)) ||
-							res == CURLE_SEND_ERROR)
+								(eCURL_ABORT_REASON_LOW_BANDWIDTH_TIMEDOUT != abortReason));
+						if (IsRetryableCurlFailure(res) || IsCurlTimeoutFailure(res) ||
+							isRetryableStall)
 						{
 
 						if(mpStreamAbstractionAAMP)

test/utests/fakes/FakeAampUtils.cpp

@@ -652,5 +652,9 @@ bool IsCurlTimeoutFailure( int httpResponseCode )
 {
 	return true;
 }
+bool IsRetryableCurlFailure( CURLcode curlCode )
+{
+	return true;
+}
 // aamp_ApplyPageHttpHeaders not actually part of AampUtils.cpp, but fake declared here for convenience
 extern "C" void aamp_ApplyPageHttpHeaders(PlayerInstanceAAMP *aamp){}

test/utests/tests/AampCurlDownloader/FunctionalTests.cpp

@@ -590,6 +590,74 @@ TEST_F(FunctionalTests, AampCurlDownloader_Retry_SendError)
 	EXPECT_FALSE(mAampCurlDownloader->IsDownloadActive());
 }
 
+class RetryableCurlFailureTests : public FunctionalTests,
+	public ::testing::WithParamInterface<CURLcode>
+{
+};
+
+/**
+ * @brief Verify that newly retryable curl transport failures are retried.
+ */
+TEST_P(RetryableCurlFailureTests,
+	RetryableCurlFailure_RetryAndSucceed)
+{
+	DownloadResponsePtr respData = std::make_shared<DownloadResponse>();
+	DownloadConfigPtr inpData = std::make_shared<DownloadConfig>();
+	inpData->bNeedDownloadMetrics = true;
+	inpData->bIgnoreResponseHeader = true;
+	inpData->iDownloadRetryCount = 1;
+
+	EXPECT_CALL(*g_mockCurl, curl_easy_init()).WillOnce(Return(mCurlEasyHandle));
+	/* The curl easy handle will be cleaned when AampCurlDownloader is destroyed. */
+	EXPECT_CALL(*g_mockCurl, curl_easy_cleanup(mCurlEasyHandle));
+	EXPECT_CALL(*g_mockCurl,
+		curl_easy_setopt_ptr(mCurlEasyHandle, CURLOPT_PROGRESSDATA,
+		mAampCurlDownloader)).WillOnce(Return(CURLE_OK));
+	EXPECT_CALL(*g_mockCurl,
+		curl_easy_setopt_func_xferinfo(mCurlEasyHandle,
+		CURLOPT_XFERINFOFUNCTION, NotNull()))
+		.WillOnce(DoAll(SaveArgPointee<2>(&mCurlProgressCallback),
+		Return(CURLE_OK)));
+	EXPECT_CALL(*g_mockCurl,
+		curl_easy_setopt_ptr(mCurlEasyHandle, CURLOPT_WRITEDATA,
+		mAampCurlDownloader)).WillOnce(Return(CURLE_OK));
+	EXPECT_CALL(*g_mockCurl,
+		curl_easy_setopt_func_write(mCurlEasyHandle,
+		CURLOPT_WRITEFUNCTION, NotNull()))
+		.WillOnce(DoAll(SaveArgPointee<2>(&mCurlWriteFunc),
+		Return(CURLE_OK)));
+	mAampCurlDownloader->Initialize(inpData);
+
+	ASSERT_NE(mCurlProgressCallback, nullptr);
+	ASSERT_NE(mCurlWriteFunc, nullptr);
+
+	EXPECT_CALL(*g_mockCurl,
+		curl_easy_setopt_str(mCurlEasyHandle, CURLOPT_URL, mUrl.c_str()))
+		.WillOnce(Return(CURLE_OK));
+	EXPECT_CALL(*g_mockCurl, curl_easy_perform(mCurlEasyHandle))
+		.WillOnce(Return(GetParam()))
+		.WillOnce(Return(CURLE_OK));
+	EXPECT_CALL(*g_mockCurl,
+		curl_easy_getinfo_int(mCurlEasyHandle, CURLINFO_RESPONSE_CODE,
+		NotNull()))
+		.WillOnce(DoAll(SetArgPointee<2>(200), Return(CURLE_OK)));
+
+	mAampCurlDownloader->Download(mUrl, respData);
+
+	EXPECT_EQ(200, respData->iHttpRetValue);
+	EXPECT_FALSE(mAampCurlDownloader->IsDownloadActive());
+}
+
+INSTANTIATE_TEST_SUITE_P(
+	RetryableTransportFailures,
+	RetryableCurlFailureTests,
+	::testing::Values(
+		CURLE_PARTIAL_FILE,
+		CURLE_COULDNT_RESOLVE_PROXY,
+		CURLE_COULDNT_RESOLVE_HOST,
+		CURLE_RECV_ERROR,
+		CURLE_GOT_NOTHING));
+
 /**
  * @brief Verify that downloadCompleteMetrics.total uses cumulative wall-clock time, not CURLINFO_TOTAL_TIME.
  *