diff --git a/app_flutter/lib/build_dashboard.dart b/app_flutter/lib/build_dashboard.dart index e5a46ae261..0b038a325e 100644 --- a/app_flutter/lib/build_dashboard.dart +++ b/app_flutter/lib/build_dashboard.dart @@ -49,8 +49,9 @@ class BuildDashboard extends StatelessWidget { builder: (_, FlutterBuildState buildState, Widget child) => Scaffold( appBar: AppBar( title: const Text('Flutter Build Dashboard v2'), - backgroundColor: - buildState.isTreeBuilding ? theme.primaryColor : theme.errorColor, + backgroundColor: buildState.isTreeBuilding.data + ? theme.primaryColor + : theme.errorColor, ), body: Column( children: const [ diff --git a/app_flutter/lib/service/appengine_cocoon.dart b/app_flutter/lib/service/appengine_cocoon.dart index f5d69c67c6..7583918aeb 100644 --- a/app_flutter/lib/service/appengine_cocoon.dart +++ b/app_flutter/lib/service/appengine_cocoon.dart @@ -31,40 +31,55 @@ class AppEngineCocoonService implements CocoonService { final http.Client _client; @override - Future> fetchCommitStatuses() async { + Future>> fetchCommitStatuses() async { /// This endpoint returns JSON [List, List] final http.Response response = await _client.get('$_baseApiUrl/public/get-status'); if (response.statusCode != HttpStatus.ok) { - throw HttpException( - '$_baseApiUrl/public/get-status returned ${response.statusCode}'); + print(response.body); + return CocoonResponse>() + ..error = + '$_baseApiUrl/public/get-status returned ${response.statusCode}'; } - final Map jsonResponse = jsonDecode(response.body); - - return _commitStatusesFromJson(jsonResponse['Statuses']); + try { + final Map jsonResponse = jsonDecode(response.body); + return CocoonResponse>() + ..data = _commitStatusesFromJson(jsonResponse['Statuses']); + } catch (error) { + return CocoonResponse>()..error = error.toString(); + } } @override - Future fetchTreeBuildStatus() async { + Future> fetchTreeBuildStatus() async { /// This endpoint returns JSON {AnticipatedBuildStatus: [BuildStatus]} final http.Response response = await _client.get('$_baseApiUrl/public/build-status'); if (response.statusCode != HttpStatus.ok) { - throw HttpException( - '$_baseApiUrl/public/build-status returned ${response.statusCode}'); + print(response.body); + return CocoonResponse() + ..error = + '$_baseApiUrl/public/build-status returned ${response.statusCode}'; } - final Map jsonResponse = jsonDecode(response.body); + Map jsonResponse; + try { + jsonResponse = jsonDecode(response.body); + } catch (error) { + return CocoonResponse() + ..error = '$_baseApiUrl/public/build-status had a malformed response'; + } if (!_isBuildStatusResponseValid(jsonResponse)) { - throw const HttpException( - '$_baseApiUrl/public/build-status had a malformed response'); + return CocoonResponse() + ..error = '$_baseApiUrl/public/build-status had a malformed response'; } - return jsonResponse['AnticipatedBuildStatus'] == 'Succeeded'; + return CocoonResponse() + ..data = jsonResponse['AnticipatedBuildStatus'] == 'Succeeded'; } /// Check if [Map] follows the format for build-status. diff --git a/app_flutter/lib/service/cocoon.dart b/app_flutter/lib/service/cocoon.dart index c11e8debc8..7ca9523c23 100644 --- a/app_flutter/lib/service/cocoon.dart +++ b/app_flutter/lib/service/cocoon.dart @@ -30,8 +30,19 @@ abstract class CocoonService { /// Gets build information from the last 200 commits. /// // TODO(chillers): Make configurable to get range of commits, https://github.com/flutter/cocoon/issues/458 - Future> fetchCommitStatuses(); + Future>> fetchCommitStatuses(); /// Gets the current build status of flutter/flutter. - Future fetchTreeBuildStatus(); + Future> fetchTreeBuildStatus(); +} + +/// Wrapper class for data this state serves. +/// +/// Holds [data] and possible error information. +class CocoonResponse { + /// The data that gets used from [CocoonService]. + T data; + + /// Error information that can be used for debugging. + String error; } diff --git a/app_flutter/lib/service/fake_cocoon.dart b/app_flutter/lib/service/fake_cocoon.dart index 7f2e9d68cd..474bd84dea 100644 --- a/app_flutter/lib/service/fake_cocoon.dart +++ b/app_flutter/lib/service/fake_cocoon.dart @@ -20,13 +20,14 @@ class FakeCocoonService implements CocoonService { final Random random; @override - Future> fetchCommitStatuses() async { - return _createFakeCommitStatuses(); + Future>> fetchCommitStatuses() async { + return CocoonResponse>() + ..data = _createFakeCommitStatuses(); } @override - Future fetchTreeBuildStatus() async { - return random.nextBool(); + Future> fetchTreeBuildStatus() async { + return CocoonResponse()..data = random.nextBool(); } List _createFakeCommitStatuses() { diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index 8ad36c065d..7d29843a23 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -30,11 +30,16 @@ class FlutterBuildState extends ChangeNotifier { Timer refreshTimer; /// The current status of the commits loaded. - List statuses = []; + CocoonResponse> _statuses = + CocoonResponse>()..data = []; + CocoonResponse> get statuses => _statuses; /// Whether or not flutter/flutter currently passes tests. - bool get isTreeBuilding => _isTreeBuilding; - bool _isTreeBuilding = true; + CocoonResponse _isTreeBuilding = CocoonResponse()..data = false; + CocoonResponse get isTreeBuilding => _isTreeBuilding; + + /// Whether an error occured getting the latest data for the fields of this state. + bool get hasError => _isTreeBuilding.error != null || _statuses.error != null; /// Start a fixed interval loop that fetches build state updates based on [refreshRate]. Future startFetchingBuildStateUpdates() async { @@ -50,14 +55,15 @@ class FlutterBuildState extends ChangeNotifier { Timer.periodic(refreshRate, (_) => _fetchBuildStatusUpdate()); } - /// Request the latest [statuses] from [CocoonService]. + /// Request the latest [statuses] and [isTreeBuilding] from [CocoonService]. Future _fetchBuildStatusUpdate() async { - await Future.wait(>[ - _cocoonService.fetchCommitStatuses().then>( - (List commitStatuses) => statuses = commitStatuses), + await Future.wait(>[ + _cocoonService.fetchCommitStatuses().then( + (CocoonResponse> response) => + _statuses = response), _cocoonService .fetchTreeBuildStatus() - .then((bool treeStatus) => _isTreeBuilding = treeStatus), + .then((CocoonResponse response) => _isTreeBuilding = response), ]); notifyListeners(); diff --git a/app_flutter/lib/status_grid.dart b/app_flutter/lib/status_grid.dart index eefc7a2b55..ab9c789d91 100644 --- a/app_flutter/lib/status_grid.dart +++ b/app_flutter/lib/status_grid.dart @@ -19,11 +19,22 @@ import 'task_icon.dart'; class StatusGridContainer extends StatelessWidget { const StatusGridContainer({Key key}) : super(key: key); + @visibleForTesting + static const String errorCocoonBackend = 'Cocoon Backend is having issues'; + @override Widget build(BuildContext context) { return Consumer( builder: (_, FlutterBuildState buildState, Widget child) { - final List statuses = buildState.statuses; + final List statuses = buildState.statuses.data; + + if (buildState.hasError) { + print('FlutterBuildState has an error'); + print('isTreeBuilding: ${buildState.isTreeBuilding.error}'); + print('statuses: ${buildState.statuses.error}'); + + // TODO(chillers): Display the error + } // Assume if there is no data that it is loading. if (statuses.isEmpty) { diff --git a/app_flutter/test/service/appengine_cocoon_test.dart b/app_flutter/test/service/appengine_cocoon_test.dart index de6e409d53..79c028a5ef 100644 --- a/app_flutter/test/service/appengine_cocoon_test.dart +++ b/app_flutter/test/service/appengine_cocoon_test.dart @@ -7,10 +7,12 @@ import 'package:http/http.dart' show Request, Response; import 'package:http/testing.dart'; import 'package:test/test.dart'; -import 'package:app_flutter/service/appengine_cocoon.dart'; import 'package:cocoon_service/protos.dart' show Commit, CommitStatus, Stage, Task; +import 'package:app_flutter/service/appengine_cocoon.dart'; +import 'package:app_flutter/service/cocoon.dart'; + // This is based off data the Cocoon backend sends out from v1. // It doesn't map directly to protos since the backend does // not use protos yet. @@ -86,13 +88,14 @@ void main() { })); }); - test('should return List', () { + test('should return CocoonResponse>', () { expect(service.fetchCommitStatuses(), - const TypeMatcher>>()); + const TypeMatcher>>>()); }); test('should return expected List', () async { - final List statuses = await service.fetchCommitStatuses(); + final CocoonResponse> statuses = + await service.fetchCommitStatuses(); final CommitStatus expectedStatus = CommitStatus() ..commit = (Commit() @@ -118,22 +121,26 @@ void main() { ..stageName = 'devicelab' ..status = 'Succeeded')); - expect(statuses.length, 1); - expect(statuses.first, expectedStatus); + expect(statuses.data.length, 1); + expect(statuses.data.first, expectedStatus); }); - test('should throw exception if given non-200 response', () { + test('should have error if given non-200 response', () async { service = AppEngineCocoonService( client: MockClient((Request request) async => Response('', 404))); - expect(service.fetchCommitStatuses(), throwsException); + final CocoonResponse> response = + await service.fetchCommitStatuses(); + expect(response.error, isNotNull); }); - test('should throw exception if given bad response', () { + test('should have error if given bad response', () async { service = AppEngineCocoonService( client: MockClient((Request request) async => Response('bad', 200))); - expect(service.fetchCommitStatuses(), throwsException); + final CocoonResponse> response = + await service.fetchCommitStatuses(); + expect(response.error, isNotNull); }); }); @@ -147,39 +154,46 @@ void main() { })); }); - test('should return bool', () { - expect(service.fetchTreeBuildStatus(), const TypeMatcher>()); + test('should return CocoonResponse', () { + expect(service.fetchTreeBuildStatus(), + const TypeMatcher>>()); }); - test('should return true when given Succeeded', () async { - final bool treeBuildStatus = await service.fetchTreeBuildStatus(); + test('data should be true when given Succeeded', () async { + final CocoonResponse treeBuildStatus = + await service.fetchTreeBuildStatus(); - expect(treeBuildStatus, true); + expect(treeBuildStatus.data, true); }); - test('should return false when given Failed', () async { + test('data should be false when given Failed', () async { service = AppEngineCocoonService(client: MockClient((Request request) async { return Response(jsonBuildStatusFalseResponse, 200); })); - final bool treeBuildStatus = await service.fetchTreeBuildStatus(); + final CocoonResponse treeBuildStatus = + await service.fetchTreeBuildStatus(); - expect(treeBuildStatus, false); + expect(treeBuildStatus.data, false); }); - test('should throw exception if given non-200 response', () { + test('should have error if given non-200 response', () async { service = AppEngineCocoonService( client: MockClient((Request request) async => Response('', 404))); - expect(service.fetchTreeBuildStatus(), throwsException); + final CocoonResponse response = + await service.fetchTreeBuildStatus(); + expect(response.error, isNotNull); }); - test('should throw exception if given bad response', () { + test('should have error if given bad response', () async { service = AppEngineCocoonService( client: MockClient((Request request) async => Response('bad', 200))); - expect(service.fetchTreeBuildStatus(), throwsException); + final CocoonResponse response = + await service.fetchTreeBuildStatus(); + expect(response.error, isNotNull); }); }); } diff --git a/app_flutter/test/state/flutter_build_test.dart b/app_flutter/test/state/flutter_build_test.dart index 3f83f129b9..dfa2b4fe10 100644 --- a/app_flutter/test/state/flutter_build_test.dart +++ b/app_flutter/test/state/flutter_build_test.dart @@ -9,6 +9,7 @@ import 'package:mockito/mockito.dart'; import 'package:cocoon_service/protos.dart' show CommitStatus; +import 'package:app_flutter/service/cocoon.dart'; import 'package:app_flutter/service/fake_cocoon.dart'; import 'package:app_flutter/state/flutter_build.dart'; @@ -21,10 +22,12 @@ void main() { mockService = MockCocoonService(); buildState = FlutterBuildState(cocoonService: mockService); - when(mockService.fetchCommitStatuses()).thenAnswer( - (_) => Future>.value([])); - when(mockService.fetchTreeBuildStatus()) - .thenAnswer((_) => Future.value(true)); + when(mockService.fetchCommitStatuses()).thenAnswer((_) => + Future>>.value( + CocoonResponse>()..data = [])); + when(mockService.fetchTreeBuildStatus()).thenAnswer((_) => + Future>.value( + CocoonResponse()..data = true)); }); testWidgets('timer should periodically fetch updates', diff --git a/app_flutter/test/status_grid_test.dart b/app_flutter/test/status_grid_test.dart index 213052dc62..c65bd26aa3 100644 --- a/app_flutter/test/status_grid_test.dart +++ b/app_flutter/test/status_grid_test.dart @@ -8,6 +8,7 @@ import 'package:provider/provider.dart'; import 'package:cocoon_service/protos.dart' show CommitStatus, Task; +import 'package:app_flutter/service/cocoon.dart'; import 'package:app_flutter/service/fake_cocoon.dart'; import 'package:app_flutter/state/flutter_build.dart'; import 'package:app_flutter/commit_box.dart'; @@ -20,7 +21,9 @@ void main() { setUpAll(() async { final FakeCocoonService service = FakeCocoonService(); - statuses = await service.fetchCommitStatuses(); + final CocoonResponse> response = + await service.fetchCommitStatuses(); + statuses = response.data; }); testWidgets('shows loading indicator when statuses is empty',