Skip to content

Commit 4914f01

Browse files
authored
Use new HTTP retry logic when fetching SDK's index.json + refactor (#8711)
1 parent bd68aa0 commit 4914f01

File tree

7 files changed

+82
-46
lines changed

7 files changed

+82
-46
lines changed

app/lib/search/backend.dart

Lines changed: 32 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ void registerSearchIndex(SearchIndex index) =>
8787
class SearchBackend {
8888
final DatastoreDB _db;
8989
final VersionedJsonStorage _snapshotStorage;
90-
final _http = httpRetryClient();
9190

9291
SearchBackend(this._db, Bucket snapshotBucket)
9392
: _snapshotStorage = VersionedJsonStorage(snapshotBucket, 'snapshot/');
@@ -428,42 +427,6 @@ class SearchBackend {
428427
}
429428
}
430429

431-
/// Downloads the remote SDK content from [uri] and creates a cached file in the
432-
/// `.dart_tool/pub-search-data/` directory.
433-
///
434-
/// When a local file in `app/.dart_tool/pub-search-data/<reduced-uri>` exists,
435-
/// its content will be loaded instead. URL reduction replaces slashes and other
436-
/// non-characters with a single dash `-`, like:
437-
/// - `https-api.dart.dev-stable-latest-index.json`
438-
Future<String> loadOrFetchSdkIndexJsonAsString(
439-
Uri uri, {
440-
@visibleForTesting Duration? ttl,
441-
}) async {
442-
final fileName = uri.toString().replaceAll(RegExp(r'[^a-z0-9\.]+'), '-');
443-
final file = File(p.join('.dart_tool', 'pub-search-data', fileName));
444-
if (await file.exists()) {
445-
var canUseCached = true;
446-
if (ttl != null) {
447-
final age = clock.now().difference(await file.lastModified());
448-
if (age > ttl) {
449-
canUseCached = false;
450-
}
451-
}
452-
if (canUseCached) {
453-
return await file.readAsString();
454-
}
455-
}
456-
457-
final rs = await _http.get(uri);
458-
if (rs.statusCode != 200) {
459-
throw Exception('Unexpected status code for $uri: ${rs.statusCode}');
460-
}
461-
final content = rs.body;
462-
await file.parent.create(recursive: true);
463-
await file.writeAsString(content);
464-
return content;
465-
}
466-
467430
Future<List<PackageDocument>?> fetchSnapshotDocuments() async {
468431
try {
469432
final map = await _snapshotStorage.getContentAsJsonMap();
@@ -519,7 +482,6 @@ class SearchBackend {
519482

520483
Future<void> close() async {
521484
_snapshotStorage.close();
522-
_http.close();
523485
}
524486
}
525487

@@ -615,6 +577,38 @@ List<ApiDocPage> apiDocPagesFromPubData(PubDartdocData pubData) {
615577
return results;
616578
}
617579

580+
/// Downloads the remote SDK content from [uri] and creates a cached file in the
581+
/// `.dart_tool/pub-search-data/` directory.
582+
///
583+
/// When a local file in `app/.dart_tool/pub-search-data/<reduced-uri>` exists,
584+
/// its content will be loaded instead. URL reduction replaces slashes and other
585+
/// non-characters with a single dash `-`, like:
586+
/// - `https-api.dart.dev-stable-latest-index.json`
587+
Future<String> loadOrFetchSdkIndexJsonAsString(
588+
Uri uri, {
589+
@visibleForTesting Duration? ttl,
590+
}) async {
591+
final fileName = uri.toString().replaceAll(RegExp(r'[^a-z0-9\.]+'), '-');
592+
final file = File(p.join('.dart_tool', 'pub-search-data', fileName));
593+
if (await file.exists()) {
594+
var canUseCached = true;
595+
if (ttl != null) {
596+
final age = clock.now().difference(await file.lastModified());
597+
if (age > ttl) {
598+
canUseCached = false;
599+
}
600+
}
601+
if (canUseCached) {
602+
return await file.readAsString();
603+
}
604+
}
605+
606+
final content = await httpGetWithRetry(uri, responseFn: (rs) => rs.body);
607+
await file.parent.create(recursive: true);
608+
await file.writeAsString(content);
609+
return content;
610+
}
611+
618612
class _CombinedSearchIndex implements SearchIndex {
619613
const _CombinedSearchIndex();
620614

app/lib/search/dart_sdk_mem_index.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ SdkMemIndex? get dartSdkMemIndex =>
3838
Future<SdkMemIndex?> createDartSdkMemIndex() async {
3939
try {
4040
final index = await SdkMemIndex.dart();
41-
final content =
42-
await searchBackend.loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
41+
final content = await loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
4342
await index.addDartdocIndex(DartdocIndex.parseJsonText(content));
4443
index.updateWeights(
4544
libraryWeights: dartSdkLibraryWeights,

app/lib/search/flutter_sdk_mem_index.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ SdkMemIndex? get flutterSdkMemIndex =>
5555
Future<SdkMemIndex?> createFlutterSdkMemIndex() async {
5656
try {
5757
final index = SdkMemIndex.flutter();
58-
final content =
59-
await searchBackend.loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
58+
final content = await loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
6059
await index.addDartdocIndex(DartdocIndex.parseJsonText(content),
6160
allowedLibraries: _allowedLibraries);
6261
index.updateWeights(

app/test/search/backend_test.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ void main() {
1414
group('search backend', () {
1515
testWithProfile('fetch SDK library description', fn: () async {
1616
final index = await SdkMemIndex.dart();
17-
final content = await searchBackend
18-
.loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
17+
final content = await loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
1918
await index.addDartdocIndex(DartdocIndex.parseJsonText(content));
2019
expect(
2120
index.getLibraryDescription('dart:async'),

app/test/search/dartdoc_index_parsing_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import '../shared/test_services.dart';
1313
void main() {
1414
group('dartdoc index.json parsing', () {
1515
testWithProfile('parse Dart SDK index.json', fn: () async {
16-
final textContent = await searchBackend.loadOrFetchSdkIndexJsonAsString(
16+
final textContent = await loadOrFetchSdkIndexJsonAsString(
1717
Uri.parse('https://api.dart.dev/stable/latest/index.json'),
1818
ttl: Duration(days: 1),
1919
);
@@ -35,7 +35,7 @@ void main() {
3535
});
3636

3737
testWithProfile('parse Flutter SDK index.json', fn: () async {
38-
final textContent = await searchBackend.loadOrFetchSdkIndexJsonAsString(
38+
final textContent = await loadOrFetchSdkIndexJsonAsString(
3939
Uri.parse('https://api.flutter.dev/flutter/index.json'),
4040
ttl: Duration(days: 1),
4141
);

pkg/_pub_shared/lib/utils/http.dart

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

5+
import 'dart:async';
56
import 'dart:io';
67

78
import 'package:http/http.dart' as http;
89
import 'package:http/retry.dart';
10+
import 'package:retry/retry.dart';
911

1012
final _transientStatusCodes = {
1113
// See: https://cloud.google.com/storage/docs/xml-api/reference-status
@@ -35,3 +37,45 @@ http.Client httpRetryClient({
3537
whenError: (e, st) => lenient || e is SocketException,
3638
);
3739
}
40+
41+
/// Creates a HTTP client and executes a GET request to the specified [uri],
42+
/// making sure that the HTTP resources are freed after the [responseFn]
43+
/// callback finishes.
44+
/// The HTTP GET and the [responseFn] callback is retried on the transient
45+
/// network errors.
46+
Future<K> httpGetWithRetry<K>(
47+
Uri uri, {
48+
required FutureOr<K> Function(http.Response response) responseFn,
49+
int maxAttempts = 3,
50+
}) async {
51+
return await retry(
52+
() async {
53+
final client = http.Client();
54+
try {
55+
final rs = await client.get(uri);
56+
if (rs.statusCode == 200) {
57+
return responseFn(rs);
58+
}
59+
throw http.ClientException(
60+
'Unexpected status code for $uri: ${rs.statusCode}.');
61+
} finally {
62+
client.close();
63+
}
64+
},
65+
maxAttempts: maxAttempts,
66+
retryIf: _retryIf,
67+
);
68+
}
69+
70+
bool _retryIf(Exception e) {
71+
if (e is TimeoutException) {
72+
return true; // Timeouts we can retry
73+
}
74+
if (e is IOException) {
75+
return true; // I/O issues are worth retrying
76+
}
77+
if (e is http.ClientException) {
78+
return true; // HTTP issues are worth retrying
79+
}
80+
return false;
81+
}

pkg/_pub_shared/pubspec.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ dependencies:
1313
logging: ^1.2.0
1414
meta: ^1.3.0
1515
pub_semver: ^2.0.0
16+
retry: ^3.1.2
1617
sanitize_html: ^2.1.0
1718
api_builder:
1819

0 commit comments

Comments
 (0)