Adding a timeout and retry to upload results step. (#87306)

* Adding a timeout and retry to upload results step.

* Adding tests for retries and timeout.

* Adding async to call

* Fixing sleep to trigger the timeout, and adding addition timeout test.

* Fixing minor typo.

* Fixing some minor flutter style issues.
Co-authored-by: yusufm <mohsinally@google.com>

dev/devicelab/lib/framework/cocoon.dart

@@ -37,6 +37,7 @@ class Cocoon {
     @visibleForTesting this.fs = const LocalFileSystem(),
     @visibleForTesting this.processRunSync = Process.runSync,
     @visibleForTesting this.requestRetryLimit = 5,
+    @visibleForTesting this.requestTimeoutLimit = 30,
   }) : _httpClient = AuthenticatedCocoonClient(serviceAccountTokenPath, httpClient: httpClient, filesystem: fs);
 
   /// Client to make http requests to Cocoon.
@@ -58,6 +59,9 @@ class Cocoon {
   @visibleForTesting
   final int requestRetryLimit;
 
+  @visibleForTesting
+  final int requestTimeoutLimit;
+
   String get commitSha => _commitSha ?? _readCommitSha();
   String? _commitSha;
 
@@ -99,8 +103,12 @@ class Cocoon {
     }
     resultsJson['TestFlaky'] = isTestFlaky ?? false;
     const List<String> supportedBranches = <String>['master'];
-    if(supportedBranches.contains(resultsJson['CommitBranch'])) {
-      await _sendUpdateTaskRequest(resultsJson);
+    if (supportedBranches.contains(resultsJson['CommitBranch'])) {
+      await retry(
+        () async => _sendUpdateTaskRequest(resultsJson).timeout(Duration(seconds: requestTimeoutLimit)),
+        retryIf: (Exception e) => e is SocketException || e is TimeoutException || e is ClientException,
+        maxAttempts: requestRetryLimit,
+      );
     }
   }
 
@@ -159,6 +167,7 @@ class Cocoon {
   }
 
   Future<void> _sendUpdateTaskRequest(Map<String, dynamic> postBody) async {
+    logger.info('Attempting to send update task request to Cocoon.');
     final Map<String, dynamic> response = await _sendCocoonRequest('update-task-status', postBody);
     if (response['Name'] != null) {
       logger.info('Updated Cocoon with results from this task');

dev/devicelab/test/cocoon_test.dart

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
@@ -156,6 +157,140 @@ void main() {
       await cocoon.sendResultsPath(resultsPath: resultsPath);
     });
 
+    test('Verify retries for task result upload', () async {
+      int requestCount = 0;
+      mockClient = MockClient((Request request) async {
+        requestCount++;
+        if (requestCount == 1) {
+          return Response('{}', 500);
+        } else {
+          return Response('{}', 200);
+        }
+      });
+
+      _processResult = ProcessResult(1, 0, commitSha, '');
+      cocoon = Cocoon(
+        fs: fs,
+        httpClient: mockClient,
+        processRunSync: runSyncStub,
+        serviceAccountTokenPath: serviceAccountTokenPath,
+        requestRetryLimit: 3,
+      );
+
+      const String resultsPath = 'results.json';
+      const String updateTaskJson = '{'
+          '"CommitBranch":"master",'
+          '"CommitSha":"$commitSha",'
+          '"BuilderName":"builderAbc",'
+          '"NewStatus":"Succeeded",'
+          '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},'
+          '"BenchmarkScoreKeys":["i","j"]}';
+      fs.file(resultsPath).writeAsStringSync(updateTaskJson);
+      await cocoon.sendResultsPath(resultsPath: resultsPath);
+    });
+
+    test('Verify timeout and retry for task result upload', () async {
+      int requestCount = 0;
+      const int timeoutValue = 2;
+      mockClient = MockClient((Request request) async {
+        requestCount++;
+        if (requestCount == 1) {
+          await Future<void>.delayed(const Duration(seconds: timeoutValue + 2));
+          throw Exception('Should not reach this, because timeout should trigger');
+        } else {
+          return Response('{}', 200);
+        }
+      });
+
+      _processResult = ProcessResult(1, 0, commitSha, '');
+      cocoon = Cocoon(
+        fs: fs,
+        httpClient: mockClient,
+        processRunSync: runSyncStub,
+        serviceAccountTokenPath: serviceAccountTokenPath,
+        requestRetryLimit: 2,
+        requestTimeoutLimit: timeoutValue,
+      );
+
+      const String resultsPath = 'results.json';
+      const String updateTaskJson = '{'
+          '"CommitBranch":"master",'
+          '"CommitSha":"$commitSha",'
+          '"BuilderName":"builderAbc",'
+          '"NewStatus":"Succeeded",'
+          '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},'
+          '"BenchmarkScoreKeys":["i","j"]}';
+      fs.file(resultsPath).writeAsStringSync(updateTaskJson);
+      await cocoon.sendResultsPath(resultsPath: resultsPath);
+    });
+
+    test('Verify timeout does not trigger for result upload', () async {
+      int requestCount = 0;
+      const int timeoutValue = 2;
+      mockClient = MockClient((Request request) async {
+        requestCount++;
+        if (requestCount == 1) {
+          await Future<void>.delayed(const Duration(seconds: timeoutValue - 1));
+          return Response('{}', 200);
+        } else {
+          throw Exception('This iteration should not be reached, since timeout should not happen.');
+        }
+      });
+
+      _processResult = ProcessResult(1, 0, commitSha, '');
+      cocoon = Cocoon(
+        fs: fs,
+        httpClient: mockClient,
+        processRunSync: runSyncStub,
+        serviceAccountTokenPath: serviceAccountTokenPath,
+        requestRetryLimit: 2,
+        requestTimeoutLimit: timeoutValue,
+      );
+
+      const String resultsPath = 'results.json';
+      const String updateTaskJson = '{'
+          '"CommitBranch":"master",'
+          '"CommitSha":"$commitSha",'
+          '"BuilderName":"builderAbc",'
+          '"NewStatus":"Succeeded",'
+          '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},'
+          '"BenchmarkScoreKeys":["i","j"]}';
+      fs.file(resultsPath).writeAsStringSync(updateTaskJson);
+      await cocoon.sendResultsPath(resultsPath: resultsPath);
+    });
+
+    test('Verify failure without retries for task result upload', () async {
+      int requestCount = 0;
+      mockClient = MockClient((Request request) async {
+        requestCount++;
+        if (requestCount == 1) {
+          return Response('{}', 500);
+        } else {
+          return Response('{}', 200);
+        }
+      });
+
+      _processResult = ProcessResult(1, 0, commitSha, '');
+      cocoon = Cocoon(
+        fs: fs,
+        httpClient: mockClient,
+        processRunSync: runSyncStub,
+        serviceAccountTokenPath: serviceAccountTokenPath,
+        requestRetryLimit: 0,
+      );
+
+      const String resultsPath = 'results.json';
+      const String updateTaskJson = '{'
+          '"CommitBranch":"master",'
+          '"CommitSha":"$commitSha",'
+          '"BuilderName":"builderAbc",'
+          '"NewStatus":"Succeeded",'
+          '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},'
+          '"BenchmarkScoreKeys":["i","j"]}';
+      fs.file(resultsPath).writeAsStringSync(updateTaskJson);
+      expect(() => cocoon.sendResultsPath(resultsPath: resultsPath), throwsA(isA<ClientException>()));
+    });
+
     test('throws client exception on non-200 responses', () async {
       mockClient = MockClient((Request request) async => Response('', 500));
 
@@ -175,8 +310,7 @@ void main() {
           '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},'
           '"BenchmarkScoreKeys":["i","j"]}';
       fs.file(resultsPath).writeAsStringSync(updateTaskJson);
-      expect(() => cocoon.sendResultsPath(resultsPath: resultsPath),
-          throwsA(isA<ClientException>()));
+      expect(() => cocoon.sendResultsPath(resultsPath: resultsPath), throwsA(isA<ClientException>()));
     });
 
     test('does not upload results on non-supported branches', () async {