From f671c96f84e884f1ef546c9b00e71f300ae0e65e Mon Sep 17 00:00:00 2001 From: Vitor Vieira <155513369+VitorVieiraZ@users.noreply.github.com> Date: Mon, 20 Jan 2025 14:39:05 -0300 Subject: [PATCH] Retry failed download requests implementation (#3673) --- app/test/testmerginapi.cpp | 142 ++++++++++++++++++++++++++++++++++++- app/test/testmerginapi.h | 69 ++++++++++++++++++ core/merginapi.cpp | 111 +++++++++++++++++++---------- core/merginapi.h | 29 +++++++- 4 files changed, 312 insertions(+), 39 deletions(-) diff --git a/app/test/testmerginapi.cpp b/app/test/testmerginapi.cpp index 815563bef..8e7f648c3 100644 --- a/app/test/testmerginapi.cpp +++ b/app/test/testmerginapi.cpp @@ -2687,7 +2687,7 @@ void TestMerginApi::deleteRemoteProjectNow( MerginApi *api, const QString &proje QUrl url( api->mApiRoot + QStringLiteral( "/v2/projects/%1" ).arg( projectId ) ); request.setUrl( url ); qDebug() << "Trying to delete project " << projectName << ", id: " << projectId << " (" << url << ")"; - QNetworkReply *r = api->mManager.deleteResource( request ); + QNetworkReply *r = api->mManager->deleteResource( request ); QSignalSpy spy( r, &QNetworkReply::finished ); spy.wait( TestUtils::SHORT_REPLY ); @@ -2942,3 +2942,143 @@ void TestMerginApi::testParseVersion() QCOMPARE( major, 2024 ); QCOMPARE( minor, 4 ); } + +void TestMerginApi::testDownloadWithNetworkError() +{ + // Store original manager + QNetworkAccessManager *originalManager = mApi->networkManager(); + + QString projectName = "testDownloadRetry"; + createRemoteProject( mApiExtra, mWorkspaceName, projectName, mTestDataPath + "/" + TEST_PROJECT_NAME + "/" ); + + // Errors to test + QList errorsToTest = + { + QNetworkReply::TimeoutError, + QNetworkReply::NetworkSessionFailedError + }; + + foreach ( QNetworkReply::NetworkError networkError, errorsToTest ) + { + // Create mock manager - initially not failing + MockNetworkManager *failingManager = new MockNetworkManager( this ); + mApi->setNetworkManager( failingManager ); + + // Create signal spies + QSignalSpy startSpy( mApi, &MerginApi::pullFilesStarted ); + QSignalSpy retrySpy( mApi, &MerginApi::downloadItemRetried ); + QSignalSpy finishSpy( mApi, &MerginApi::syncProjectFinished ); + + // Trigger the current network error when download starts + connect( mApi, &MerginApi::pullFilesStarted, this, [this, failingManager, networkError]() + { + failingManager->setShouldFail( true, networkError ); + } ); + + mApi->pullProject( mWorkspaceName, projectName ); + + // Verify a transaction was created + QCOMPARE( mApi->transactions().count(), 1 ); + + // Wait for download to start and then fail + QVERIFY( startSpy.wait( TestUtils::LONG_REPLY ) ); + QVERIFY( finishSpy.wait( TestUtils::LONG_REPLY ) ); + + // Verify signals were emitted + QVERIFY( startSpy.count() > 0 ); + QVERIFY( retrySpy.count() > 0 ); + QCOMPARE( finishSpy.count(), 1 ); + + // Verify that MAX_RETRY_COUNT retry attempts were made + int maxRetries = TransactionStatus::MAX_RETRY_COUNT; + QCOMPARE( retrySpy.count(), maxRetries ); + + // Verify sync failed + QList arguments = finishSpy.takeFirst(); + QVERIFY( !arguments.at( 1 ).toBool() ); + + // Verify no local project was created + LocalProject localProject = mApi->localProjectsManager().projectFromMerginName( mWorkspaceName, projectName ); + QVERIFY( !localProject.isValid() ); + + // Disconnect all signals + disconnect( mApi, &MerginApi::pullFilesStarted, this, nullptr ); + + // Clean up + mApi->setNetworkManager( originalManager ); + delete failingManager; + } +} + +void TestMerginApi::testDownloadWithNetworkErrorRecovery() +{ + // Store original manager + QNetworkAccessManager *originalManager = mApi->networkManager(); + + QString projectName = "testDownloadRetryRecovery"; + createRemoteProject( mApiExtra, mWorkspaceName, projectName, mTestDataPath + "/" + TEST_PROJECT_NAME + "/" ); + + // Create mock manager - initially not failing + MockNetworkManager *failingManager = new MockNetworkManager( this ); + mApi->setNetworkManager( failingManager ); + + // Create signal spies + QSignalSpy startSpy( mApi, &MerginApi::pullFilesStarted ); + QSignalSpy retrySpy( mApi, &MerginApi::downloadItemRetried ); + QSignalSpy finishSpy( mApi, &MerginApi::syncProjectFinished ); + + // Counter to track retry attempts + int retryCount = 0; + QNetworkReply::NetworkError networkError = QNetworkReply::TimeoutError; + + // Reset network after two retries + connect( mApi, &MerginApi::downloadItemRetried, this, [&retryCount, failingManager, this]() + { + retryCount++; + if ( retryCount == 2 ) + { + failingManager->setShouldFail( false ); + disconnect( mApi, &MerginApi::pullFilesStarted, nullptr, nullptr ); + disconnect( mApi, &MerginApi::downloadItemRetried, nullptr, nullptr ); + } + } ); + + // Trigger network error when download starts + connect( mApi, &MerginApi::pullFilesStarted, this, [failingManager, networkError]() + { + failingManager->setShouldFail( true, networkError ); + } ); + + mApi->pullProject( mWorkspaceName, projectName ); + + // Verify a transaction was created + QCOMPARE( mApi->transactions().count(), 1 ); + + // Wait for download to start, retry twice, and then complete successfully + QVERIFY( startSpy.wait( TestUtils::LONG_REPLY ) ); + QVERIFY( finishSpy.wait( TestUtils::LONG_REPLY ) ); + + // Verify signals were emitted + QVERIFY( startSpy.count() > 0 ); + QCOMPARE( retrySpy.count(), 2 ); // Should have exactly 2 retries + QCOMPARE( finishSpy.count(), 1 ); + + // Verify sync succeeded + QList arguments = finishSpy.takeFirst(); + QVERIFY( arguments.at( 1 ).toBool() ); + + // Verify local project was created successfully + LocalProject localProject = mApi->localProjectsManager().projectFromMerginName( mWorkspaceName, projectName ); + QVERIFY( localProject.isValid() ); + + // Verify project files were downloaded correctly + QString projectDir = mApi->projectsPath() + "/" + projectName; + QStringList projectFiles = QDir( projectDir ).entryList( QDir::Files ); + QVERIFY( projectFiles.count() > 0 ); + QVERIFY( projectFiles.contains( "project.qgs" ) ); + + // Clean up + mApi->setNetworkManager( originalManager ); + delete failingManager; +} + diff --git a/app/test/testmerginapi.h b/app/test/testmerginapi.h index 25292fcbd..9a9640447 100644 --- a/app/test/testmerginapi.h +++ b/app/test/testmerginapi.h @@ -21,6 +21,73 @@ #include +class MockReply : public QNetworkReply +{ + public: + explicit MockReply( const QNetworkRequest &request, QNetworkAccessManager::Operation operation, + QObject *parent = nullptr, QNetworkReply::NetworkError errorCode = QNetworkReply::NoError ) + : QNetworkReply( parent ) + { + setRequest( request ); + setOperation( operation ); + setUrl( request.url() ); + + if ( errorCode != QNetworkReply::NoError ) + { + setError( errorCode, "Mock network failure" ); + QMetaObject::invokeMethod( this, "errorOccurred", Qt::QueuedConnection, Q_ARG( QNetworkReply::NetworkError, errorCode ) ); + } + + QMetaObject::invokeMethod( this, "finished", Qt::QueuedConnection ); + open( QIODevice::ReadOnly ); + } + + void abort() override {} + + qint64 readData( char *data, qint64 maxlen ) override + { + Q_UNUSED( data ); + Q_UNUSED( maxlen ); + return -1; + } + + qint64 bytesAvailable() const override + { + return 0; + } +}; + +class MockNetworkManager : public QNetworkAccessManager +{ + public: + explicit MockNetworkManager( QObject *parent = nullptr ) + : QNetworkAccessManager( parent ) + , mShouldFail( false ) + , mErrorCode( QNetworkReply::NoError ) + {} + + void setShouldFail( bool shouldFail, QNetworkReply::NetworkError errorCode = QNetworkReply::NoError ) + { + mShouldFail = shouldFail; + mErrorCode = errorCode; + } + + protected: + QNetworkReply *createRequest( Operation op, const QNetworkRequest &request, QIODevice *outgoingData = nullptr ) override + { + if ( mShouldFail ) + { + auto *reply = new MockReply( request, op, this, mErrorCode ); + return reply; + } + return QNetworkAccessManager::createRequest( op, request, outgoingData ); + } + + private: + bool mShouldFail; + QNetworkReply::NetworkError mErrorCode; +}; + class TestMerginApi: public QObject { Q_OBJECT @@ -40,6 +107,8 @@ class TestMerginApi: public QObject void testListProject(); void testListProjectsByName(); void testDownloadProject(); + void testDownloadWithNetworkError(); + void testDownloadWithNetworkErrorRecovery(); void testDownloadProjectSpecChars(); void testCancelDownloadProject(); void testCreateProjectTwice(); diff --git a/core/merginapi.cpp b/core/merginapi.cpp index ab5e4a618..5a6804903 100644 --- a/core/merginapi.cpp +++ b/core/merginapi.cpp @@ -49,6 +49,7 @@ MerginApi::MerginApi( LocalProjectsManager &localProjects, QObject *parent ) , mWorkspaceInfo( new MerginWorkspaceInfo ) , mSubscriptionInfo( new MerginSubscriptionInfo ) , mUserAuth( new MerginUserAuth ) + , mManager( new QNetworkAccessManager( this ) ) { // load cached data if there are any QSettings cache; @@ -199,7 +200,7 @@ QString MerginApi::listProjects( const QString &searchExpression, const QString QString requestId = CoreUtils::uuidWithoutBraces( QUuid::createUuid() ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "list projects", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, [this, requestId]() {this->listProjectsReplyFinished( requestId );} ); @@ -248,7 +249,7 @@ QString MerginApi::listProjectsByName( const QStringList &projectNames ) QString requestId = CoreUtils::uuidWithoutBraces( QUuid::createUuid() ); - QNetworkReply *reply = mManager.post( request, body.toJson() ); + QNetworkReply *reply = mManager->post( request, body.toJson() ); CoreUtils::log( "list projects by name", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, [this, requestId]() {this->listProjectsByNameReplyFinished( requestId );} ); @@ -286,8 +287,9 @@ void MerginApi::downloadNextItem( const QString &projectFullName ) request.setRawHeader( "Range", range.toUtf8() ); } - QNetworkReply *reply = mManager.get( request ); - connect( reply, &QNetworkReply::finished, this, &MerginApi::downloadItemReplyFinished ); + QNetworkReply *reply = mManager->get( request ); + connect( reply, &QNetworkReply::finished, this, [this, item]() { downloadItemReplyFinished( item ); } ); + transaction.replyPullItems.insert( reply ); CoreUtils::log( "pull " + projectFullName, QStringLiteral( "Requesting item: " ) + url.toString() + @@ -373,14 +375,12 @@ QString MerginApi::getApiKey( const QString &serverName ) return "not-secret-key"; } -void MerginApi::downloadItemReplyFinished() +void MerginApi::downloadItemReplyFinished( DownloadQueueItem item ) { QNetworkReply *r = qobject_cast( sender() ); Q_ASSERT( r ); - QString projectFullName = r->request().attribute( static_cast( AttrProjectFullName ) ).toString(); QString tempFileName = r->request().attribute( static_cast( AttrTempFileName ) ).toString(); - Q_ASSERT( mTransactionalStatus.contains( projectFullName ) ); TransactionStatus &transaction = mTransactionalStatus[projectFullName]; Q_ASSERT( transaction.replyPullItems.contains( r ) ); @@ -388,13 +388,10 @@ void MerginApi::downloadItemReplyFinished() if ( r->error() == QNetworkReply::NoError ) { QByteArray data = r->readAll(); - CoreUtils::log( "pull " + projectFullName, QStringLiteral( "Downloaded item (%1 bytes)" ).arg( data.size() ) ); - QString tempFolder = getTempProjectDir( projectFullName ); QString tempFilePath = tempFolder + "/" + tempFileName; createPathIfNotExists( tempFilePath ); - // save to a tmp file, assemble at the end QFile file( tempFilePath ); if ( file.open( QIODevice::WriteOnly ) ) @@ -406,11 +403,10 @@ void MerginApi::downloadItemReplyFinished() { CoreUtils::log( "pull " + projectFullName, "Failed to open for writing: " + file.fileName() ); } - transaction.transferedSize += data.size(); emit syncProjectStatusChanged( projectFullName, transaction.transferedSize / transaction.totalSize ); - transaction.replyPullItems.remove( r ); + r->deleteLater(); if ( !transaction.downloadQueue.isEmpty() ) @@ -418,6 +414,7 @@ void MerginApi::downloadItemReplyFinished() // one request finished, let's start another one downloadNextItem( projectFullName ); } + else if ( transaction.replyPullItems.isEmpty() ) { // nothing else to download and all requests are finished, we're done @@ -428,6 +425,20 @@ void MerginApi::downloadItemReplyFinished() // no more requests to start, but there are pending requests - let's do nothing and wait } } + else if ( transaction.retryCount < transaction.MAX_RETRY_COUNT && isRetryableNetworkError( r ) ) + { + transaction.retryCount++; + transaction.downloadQueue.append( item ); + + CoreUtils::log( "pull " + projectFullName, QStringLiteral( "Retrying download (attempt %1 of %2)" ).arg( transaction.retryCount ) + .arg( transaction.MAX_RETRY_COUNT ) ); + + downloadNextItem( projectFullName ); + + emit downloadItemRetried( projectFullName, transaction.retryCount ); + transaction.replyPullItems.remove( r ); + r->deleteLater(); + } else { QString serverMsg = extractServerErrorMsg( r->readAll() ); @@ -439,15 +450,12 @@ void MerginApi::downloadItemReplyFinished() serverMsg = r->errorString(); } CoreUtils::log( "pull " + projectFullName, QStringLiteral( "FAILED - %1. %2" ).arg( r->errorString(), serverMsg ) ); - transaction.replyPullItems.remove( r ); r->deleteLater(); - if ( !transaction.pullItemsAborting ) { // the first failed request will abort all the other pending requests too, and finish pull with error abortPullItems( projectFullName ); - // signal a networking error - we may retry int httpCode = r->attribute( QNetworkRequest::HttpStatusCodeAttribute ).toInt(); emit networkErrorOccurred( serverMsg, QStringLiteral( "Mergin API error: downloadFile" ), httpCode, projectFullName ); @@ -567,7 +575,7 @@ void MerginApi::pushFile( const QString &projectFullName, const QString &transac request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); Q_ASSERT( !transaction.replyPushFile ); - transaction.replyPushFile = mManager.post( request, data ); + transaction.replyPushFile = mManager->post( request, data ); connect( transaction.replyPushFile, &QNetworkReply::finished, this, &MerginApi::pushFileReplyFinished ); CoreUtils::log( "push " + projectFullName, QStringLiteral( "Uploading item: " ) + url.toString() ); @@ -590,7 +598,7 @@ void MerginApi::pushStart( const QString &projectFullName, const QByteArray &jso request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); Q_ASSERT( !transaction.replyPushStart ); - transaction.replyPushStart = mManager.post( request, json ); + transaction.replyPushStart = mManager->post( request, json ); connect( transaction.replyPushStart, &QNetworkReply::finished, this, &MerginApi::pushStartReplyFinished ); CoreUtils::log( "push " + projectFullName, QStringLiteral( "Starting push request: " ) + url.toString() ); @@ -653,7 +661,7 @@ void MerginApi::sendPushCancelRequest( const QString &projectFullName, const QSt request.setRawHeader( "Content-Type", "application/json" ); request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); - QNetworkReply *reply = mManager.post( request, QByteArray() ); + QNetworkReply *reply = mManager->post( request, QByteArray() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::pushCancelReplyFinished ); CoreUtils::log( "push " + projectFullName, QStringLiteral( "Requesting upload transaction cancel: " ) + url.toString() ); } @@ -707,7 +715,7 @@ void MerginApi::pushFinish( const QString &projectFullName, const QString &trans request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); Q_ASSERT( !transaction.replyPushFinish ); - transaction.replyPushFinish = mManager.post( request, QByteArray() ); + transaction.replyPushFinish = mManager->post( request, QByteArray() ); connect( transaction.replyPushFinish, &QNetworkReply::finished, this, &MerginApi::pushFinishReplyFinished ); CoreUtils::log( "push " + projectFullName, QStringLiteral( "Requesting transaction finish: " ) + transactionUUID ); @@ -803,7 +811,7 @@ void MerginApi::authorize( const QString &login, const QString &password ) jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, &MerginApi::authorizeFinished ); CoreUtils::log( "auth", QStringLiteral( "Requesting authorization: " ) + url.toString() ); } @@ -878,7 +886,7 @@ void MerginApi::registerUser( const QString &username, jsonObject.insert( QStringLiteral( "api_key" ), getApiKey( mApiRoot ) ); jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, [ = ]() { this->registrationFinished( username, password ); } ); CoreUtils::log( "auth", QStringLiteral( "Requesting registration: " ) + url.toString() ); } @@ -914,7 +922,7 @@ void MerginApi::postRegisterUser( const QString &marketingChannel, const QString jsonObject.insert( QStringLiteral( "subscribe" ), wantsNewsletter ); jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, [ = ]() { this->postRegistrationFinished(); } ); CoreUtils::log( "auth", QStringLiteral( "Requesting post-registration: " ) + url.toString() ); } @@ -940,7 +948,7 @@ void MerginApi::getUserInfo() QUrl url( urlString ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "user info", QStringLiteral( "Requesting user info: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::getUserInfoFinished ); } @@ -967,7 +975,7 @@ void MerginApi::getWorkspaceInfo() QUrl url( urlString ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "workspace info", QStringLiteral( "Requesting workspace info: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::getWorkspaceInfoReplyFinished ); } @@ -998,7 +1006,7 @@ void MerginApi::getServiceInfo() QUrl url( urlString ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); connect( reply, &QNetworkReply::finished, this, &MerginApi::getServiceInfoReplyFinished ); @@ -1103,7 +1111,7 @@ bool MerginApi::createProject( const QString &projectNamespace, const QString &p jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, &MerginApi::createProjectFinished ); CoreUtils::log( "create " + projectFullName, QStringLiteral( "Requesting project creation: " ) + url.toString() ); @@ -1123,7 +1131,7 @@ void MerginApi::deleteProject( const QString &projectNamespace, const QString &p QUrl url( mApiRoot + QStringLiteral( "/v1/project/%1" ).arg( projectFullName ) ); request.setUrl( url ); request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); - QNetworkReply *reply = mManager.deleteResource( request ); + QNetworkReply *reply = mManager->deleteResource( request ); connect( reply, &QNetworkReply::finished, this, [this, informUser]() { this->deleteProjectFinished( informUser );} ); CoreUtils::log( "delete " + projectFullName, QStringLiteral( "Requesting project deletion: " ) + url.toString() ); } @@ -1434,7 +1442,7 @@ QNetworkReply *MerginApi::getProjectInfo( const QString &projectFullName, bool w request.setUrl( url ); request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); - return mManager.get( request ); + return mManager->get( request ); } bool MerginApi::validateAuth() @@ -1653,7 +1661,7 @@ void MerginApi::pingMergin() QUrl url( mApiRoot + QStringLiteral( "/ping" ) ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "ping", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::pingMerginReplyFinished ); } @@ -2558,7 +2566,7 @@ void MerginApi::requestServerConfig( const QString &projectFullName ) request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); Q_ASSERT( !transaction.replyPullServerConfig ); - transaction.replyPullServerConfig = mManager.get( request ); + transaction.replyPullServerConfig = mManager->get( request ); connect( transaction.replyPullServerConfig, &QNetworkReply::finished, this, &MerginApi::cacheServerConfig ); CoreUtils::log( "pull " + projectFullName, QStringLiteral( "Requesting mergin config: " ) + url.toString() ); @@ -3519,7 +3527,7 @@ void MerginApi::deleteAccount() QNetworkRequest request = getDefaultRequest(); QUrl url( mApiRoot + QStringLiteral( "/v1/user" ) ); request.setUrl( url ); - QNetworkReply *reply = mManager.deleteResource( request ); + QNetworkReply *reply = mManager->deleteResource( request ); connect( reply, &QNetworkReply::finished, this, [this]() { this->deleteAccountFinished();} ); CoreUtils::log( "delete account " + mUserAuth->username(), QStringLiteral( "Requesting account deletion: " ) + url.toString() ); } @@ -3571,7 +3579,7 @@ void MerginApi::getServerConfig() QUrl url( urlString ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); connect( reply, &QNetworkReply::finished, this, &MerginApi::getServerConfigReplyFinished ); CoreUtils::log( "Config", QStringLiteral( "Requesting server configuration: " ) + url.toString() ); @@ -3687,7 +3695,7 @@ void MerginApi::listWorkspaces() QNetworkRequest request = getDefaultRequest( mUserAuth->hasAuthData() ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "list workspaces", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::listWorkspacesReplyFinished ); } @@ -3743,7 +3751,7 @@ void MerginApi::listInvitations() QNetworkRequest request = getDefaultRequest( mUserAuth->hasAuthData() ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "list invitations", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::listInvitationsReplyFinished ); } @@ -3806,7 +3814,7 @@ void MerginApi::processInvitation( const QString &uuid, bool accept ) jsonObject.insert( QStringLiteral( "accept" ), accept ); jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); CoreUtils::log( "process invitation", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::processInvitationReplyFinished ); } @@ -3868,7 +3876,7 @@ bool MerginApi::createWorkspace( const QString &workspaceName ) jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, &MerginApi::createWorkspaceReplyFinished ); CoreUtils::log( "create " + workspaceName, QStringLiteral( "Requesting workspace creation: " ) + url.toString() ); @@ -3945,3 +3953,34 @@ DownloadQueueItem::DownloadQueueItem( const QString &fp, qint64 s, int v, qint64 tempFileName = CoreUtils::uuidWithoutBraces( QUuid::createUuid() ); } +bool MerginApi::isRetryableNetworkError( QNetworkReply *reply ) +{ + Q_ASSERT( reply ); + + QNetworkReply::NetworkError err = reply->error(); + + bool isRetryableError = ( err == QNetworkReply::TimeoutError || + err == QNetworkReply::TemporaryNetworkFailureError || + err == QNetworkReply::NetworkSessionFailedError || + err == QNetworkReply::UnknownNetworkError || + err == QNetworkReply::RemoteHostClosedError || + err == QNetworkReply::ProxyConnectionClosedError || + err == QNetworkReply::ProxyTimeoutError || + err == QNetworkReply::UnknownProxyError || + err == QNetworkReply::ServiceUnavailableError ); + + return isRetryableError; +} + +void MerginApi::setNetworkManager( QNetworkAccessManager *manager ) +{ + if ( !manager ) + return; + + if ( mManager == manager ) + return; + + mManager = manager; + + emit networkManagerChanged(); +} diff --git a/core/merginapi.h b/core/merginapi.h index 0e5380bb2..4646e9361 100644 --- a/core/merginapi.h +++ b/core/merginapi.h @@ -166,6 +166,10 @@ struct TransactionStatus QList pushQueue; //!< pending list of files to push (at the end of transaction it is empty) QList pushDiffFiles; //!< these are just diff files for push - we don't remove them when pushing chunks (needed for finalization) + // retry handling + int retryCount = 0; //!< current number of retry attempts for failed network requests + static const int MAX_RETRY_COUNT = 5; //!< maximum number of retry attempts for failed network requests + QString projectDir; QByteArray projectMetadata; //!< metadata of the new project (not parsed) bool firstTimeDownload = false; //!< only for update. whether this is first time to download the project (on failure we would also remove the project folder) @@ -573,6 +577,17 @@ class MerginApi: public QObject */ bool apiSupportsWorkspaces(); + /** + * Returns the network manager used for Mergin API requests + */ + QNetworkAccessManager *networkManager() const { return mManager; } + + /** + * Sets the network manager to be used for Mergin API requests + * Function will return early if manager is null. + */ + void setNetworkManager( QNetworkAccessManager *manager ); + signals: void apiSupportsSubscriptionsChanged(); void supportsSelectiveSyncChanged(); @@ -650,6 +665,9 @@ class MerginApi: public QObject void apiSupportsWorkspacesChanged(); void serverWasUpgraded(); + void networkManagerChanged(); + + void downloadItemRetried( const QString &projectFullName, int retryCount ); private slots: void listProjectsReplyFinished( QString requestId ); @@ -657,7 +675,7 @@ class MerginApi: public QObject // Pull slots void pullInfoReplyFinished(); - void downloadItemReplyFinished(); + void downloadItemReplyFinished( DownloadQueueItem item ); void cacheServerConfig(); // Push slots @@ -785,11 +803,18 @@ class MerginApi: public QObject //! Works only when login, password and token is set in UserAuth void refreshAuthToken(); + /** + * Checks if a network error should trigger a retry attempt. + * \param reply Network reply to check for retryable errors + * \returns True if the error should trigger a retry, false otherwise + */ + bool isRetryableNetworkError( QNetworkReply *reply ); + QNetworkRequest getDefaultRequest( bool withAuth = true ); bool projectFileHasBeenUpdated( const ProjectDiff &diff ); - QNetworkAccessManager mManager; + QNetworkAccessManager *mManager = nullptr; QString mApiRoot; LocalProjectsManager &mLocalProjects; QString mDataDir; // dir with all projects