Skip to content

Commit 39f906c

Browse files
authored
fix: SocketException+HttpException mapped correctly while running natively (#4993)
gae_server and local_server now use a BaseClient that catches the exception and return the cocoon_service ones.
1 parent fd286b0 commit 39f906c

7 files changed

Lines changed: 363 additions & 15 deletions

File tree

app_dart/bin/gae_server.dart

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
import 'dart:io';
5+
import 'dart:io' as io;
66

77
import 'package:appengine/appengine.dart';
88
import 'package:cocoon_server/google_auth_provider.dart';
@@ -20,6 +20,7 @@ import 'package:cocoon_service/src/service/firebase_jwt_validator.dart';
2020
import 'package:cocoon_service/src/service/flags/dynamic_config_updater.dart';
2121
import 'package:cocoon_service/src/service/get_files_changed.dart';
2222
import 'package:cocoon_service/src/service/scheduler/ci_yaml_fetcher.dart';
23+
import 'package:http/http.dart' as http;
2324
import 'package:logging/logging.dart';
2425

2526
Future<void> main() async {
@@ -53,6 +54,7 @@ Future<void> main() async {
5354
projectId: Config.flutterGcpProjectId,
5455
),
5556
initialConfig: dynamicConfig,
57+
httpClient: MappingHttpClient(http.Client()),
5658
);
5759
// Start updating the config to loop forever. If this fails, it will log
5860
// every ~1 minute.
@@ -75,12 +77,16 @@ Future<void> main() async {
7577

7678
final buildBucketClient = BuildBucketClient(
7779
accessTokenService: AccessTokenService.defaultProvider(config),
80+
httpClient: config.httpClient,
7881
);
7982

8083
// Gerrit service class to communicate with GoB.
8184
final gerritService = GerritService(
8285
config: config,
83-
authClient: await const GoogleAuthProvider().createClient(scopes: []),
86+
authClient: await const GoogleAuthProvider().createClient(
87+
baseClient: config.httpClient,
88+
scopes: [],
89+
),
8490
);
8591

8692
/// LUCI service class to communicate with buildBucket service.
@@ -155,10 +161,10 @@ Future<void> main() async {
155161
);
156162

157163
return runAppEngine(
158-
(HttpRequest request) async {
164+
(io.HttpRequest request) async {
159165
await server(request.toRequest());
160166
},
161-
onAcceptingConnections: (InternetAddress address, int port) {
167+
onAcceptingConnections: (io.InternetAddress address, int port) {
162168
final host = address.isLoopback ? 'localhost' : address.host;
163169
print('Serving requests at http://$host:$port/');
164170
},

app_dart/lib/src/request_handling/http_io.dart

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ import 'dart:io';
77
import 'dart:typed_data';
88

99
import 'package:cocoon_common/core_extensions.dart';
10+
import 'package:http/http.dart' as http;
1011

11-
import 'http_utils.dart';
12+
import 'http_utils.dart' as http_utils;
1213
import 'request_handler.dart';
1314

1415
/// Creates a [Request] by wrapping an existing [HttpRequest].
@@ -74,7 +75,7 @@ final class _HttpResponse implements RequestResponse {
7475
set statusCode(int value) => _response.statusCode = value;
7576

7677
@override
77-
set contentType(MediaType? value) {
78+
set contentType(http_utils.MediaType? value) {
7879
if (value != null) {
7980
_response.headers.contentType = ContentType(
8081
value.type,
@@ -99,6 +100,30 @@ final class _HttpResponse implements RequestResponse {
99100
@override
100101
Future<dynamic> redirect(
101102
Uri location, {
102-
int status = HttpStatus.movedTemporarily,
103+
int status = http_utils.HttpStatus.movedTemporarily,
103104
}) => _response.redirect(location, status: status);
104105
}
106+
107+
/// An [http.Client] that maps [SocketException] from dart:io to the cocoon
108+
/// [http_utils.SocketException].
109+
///
110+
/// This is used to maintain platform neutrality in the core of app_dart.
111+
class MappingHttpClient extends http.BaseClient {
112+
MappingHttpClient(this._inner);
113+
114+
final http.Client _inner;
115+
116+
@override
117+
Future<http.StreamedResponse> send(http.BaseRequest request) async {
118+
try {
119+
return await _inner.send(request);
120+
} on SocketException catch (e) {
121+
throw http_utils.SocketException('$e');
122+
} on HttpException catch (e) {
123+
throw http_utils.HttpException('$e');
124+
}
125+
}
126+
127+
@override
128+
void close() => _inner.close();
129+
}

app_dart/lib/src/service/config.dart

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ const String kDefaultBranchName = 'master';
2424

2525
interface class Config extends DynamicallyUpdatedConfig {
2626
/// Creates and returns a [Config] instance.
27-
Config(this._cache, this._secrets, {required super.initialConfig});
27+
Config(
28+
this._cache,
29+
this._secrets, {
30+
required super.initialConfig,
31+
http.Client? httpClient,
32+
}) : _httpClient = httpClient ?? http.Client();
2833

2934
/// When present on a pull request, instructs Cocoon to submit it
3035
/// automatically as soon as all the required checks pass.
@@ -78,6 +83,10 @@ interface class Config extends DynamicallyUpdatedConfig {
7883

7984
final CacheService _cache;
8085
final SecretManager _secrets;
86+
final http.Client _httpClient;
87+
88+
/// The [http.Client] to use for requests.
89+
http.Client get httpClient => _httpClient;
8190

8291
/// List of Github presubmit supported repos.
8392
///
@@ -453,7 +462,10 @@ interface class Config extends DynamicallyUpdatedConfig {
453462
'api.github.com',
454463
'app/installations/$appInstallation/access_tokens',
455464
);
456-
final response = await http.post(githubAccessTokensUri, headers: headers);
465+
final response = await _httpClient.post(
466+
githubAccessTokensUri,
467+
headers: headers,
468+
);
457469
final jsonBody = jsonDecode(response.body) as Map<String, dynamic>;
458470
if (jsonBody.containsKey('token') == false) {
459471
log.warn(response.body);
@@ -476,12 +488,16 @@ interface class Config extends DynamicallyUpdatedConfig {
476488
}
477489

478490
gh.GitHub createGitHubClientWithToken(String token) {
479-
return gh.GitHub(auth: gh.Authentication.withToken(token));
491+
return gh.GitHub(
492+
auth: gh.Authentication.withToken(token),
493+
client: _httpClient,
494+
);
480495
}
481496

482497
Future<GraphQLClient> createGitHubGraphQLClient() async {
483498
final httpLink = HttpLink(
484499
'https://api.github.com/graphql',
500+
httpClient: _httpClient,
485501
defaultHeaders: <String, String>{
486502
'Accept': 'application/vnd.github.antiope-preview+json',
487503
},
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Copyright 2026 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:io' as io;
6+
7+
import 'package:cocoon_server_test/test_logging.dart';
8+
import 'package:cocoon_service/src/request_handling/http_io.dart';
9+
import 'package:cocoon_service/src/request_handling/http_utils.dart'
10+
as cocoon_service;
11+
import 'package:http/http.dart' as http;
12+
import 'package:mockito/annotations.dart';
13+
import 'package:mockito/mockito.dart';
14+
import 'package:test/test.dart';
15+
16+
import 'mapping_http_client_test.mocks.dart';
17+
18+
@GenerateMocks([http.Client])
19+
void main() {
20+
useTestLoggerPerTest();
21+
22+
group('MappingHttpClient', () {
23+
late MockClient mockClient;
24+
late MappingHttpClient mappingClient;
25+
26+
setUp(() {
27+
mockClient = MockClient();
28+
mappingClient = MappingHttpClient(mockClient);
29+
});
30+
31+
test('maps io.SocketException to cocoon_service.SocketException', () async {
32+
when(
33+
mockClient.send(any),
34+
).thenThrow(const io.SocketException('test message'));
35+
36+
expect(
37+
() => mappingClient.get(Uri.parse('https://example.com')),
38+
throwsA(
39+
isA<cocoon_service.SocketException>().having(
40+
(e) => e.message,
41+
'message',
42+
'SocketException: test message',
43+
),
44+
),
45+
);
46+
});
47+
48+
test('maps io.HttpException to cocoon_service.HttpException', () async {
49+
when(
50+
mockClient.send(any),
51+
).thenThrow(const io.HttpException('test message'));
52+
53+
expect(
54+
() => mappingClient.get(Uri.parse('https://example.com')),
55+
throwsA(
56+
isA<cocoon_service.HttpException>().having(
57+
(e) => e.message,
58+
'message',
59+
'HttpException: test message',
60+
),
61+
),
62+
);
63+
});
64+
65+
test('passes through successful responses', () async {
66+
final response = http.StreamedResponse(const Stream.empty(), 200);
67+
when(mockClient.send(any)).thenAnswer((_) async => response);
68+
69+
final result = await mappingClient.get(Uri.parse('https://example.com'));
70+
expect(result.statusCode, 200);
71+
});
72+
});
73+
}

0 commit comments

Comments
 (0)