Skip to content

Use new HTTP retry logic when fetching SDK's index.json + refactor #8711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 32 additions & 38 deletions app/lib/search/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ void registerSearchIndex(SearchIndex index) =>
class SearchBackend {
final DatastoreDB _db;
final VersionedJsonStorage _snapshotStorage;
final _http = httpRetryClient();

SearchBackend(this._db, Bucket snapshotBucket)
: _snapshotStorage = VersionedJsonStorage(snapshotBucket, 'snapshot/');
Expand Down Expand Up @@ -428,42 +427,6 @@ class SearchBackend {
}
}

/// Downloads the remote SDK content from [uri] and creates a cached file in the
/// `.dart_tool/pub-search-data/` directory.
///
/// When a local file in `app/.dart_tool/pub-search-data/<reduced-uri>` exists,
/// its content will be loaded instead. URL reduction replaces slashes and other
/// non-characters with a single dash `-`, like:
/// - `https-api.dart.dev-stable-latest-index.json`
Future<String> loadOrFetchSdkIndexJsonAsString(
Uri uri, {
@visibleForTesting Duration? ttl,
}) async {
final fileName = uri.toString().replaceAll(RegExp(r'[^a-z0-9\.]+'), '-');
final file = File(p.join('.dart_tool', 'pub-search-data', fileName));
if (await file.exists()) {
var canUseCached = true;
if (ttl != null) {
final age = clock.now().difference(await file.lastModified());
if (age > ttl) {
canUseCached = false;
}
}
if (canUseCached) {
return await file.readAsString();
}
}

final rs = await _http.get(uri);
if (rs.statusCode != 200) {
throw Exception('Unexpected status code for $uri: ${rs.statusCode}');
}
final content = rs.body;
await file.parent.create(recursive: true);
await file.writeAsString(content);
return content;
}

Future<List<PackageDocument>?> fetchSnapshotDocuments() async {
try {
final map = await _snapshotStorage.getContentAsJsonMap();
Expand Down Expand Up @@ -519,7 +482,6 @@ class SearchBackend {

Future<void> close() async {
_snapshotStorage.close();
_http.close();
}
}

Expand Down Expand Up @@ -615,6 +577,38 @@ List<ApiDocPage> apiDocPagesFromPubData(PubDartdocData pubData) {
return results;
}

/// Downloads the remote SDK content from [uri] and creates a cached file in the
/// `.dart_tool/pub-search-data/` directory.
///
/// When a local file in `app/.dart_tool/pub-search-data/<reduced-uri>` exists,
/// its content will be loaded instead. URL reduction replaces slashes and other
/// non-characters with a single dash `-`, like:
/// - `https-api.dart.dev-stable-latest-index.json`
Future<String> loadOrFetchSdkIndexJsonAsString(
Uri uri, {
@visibleForTesting Duration? ttl,
}) async {
final fileName = uri.toString().replaceAll(RegExp(r'[^a-z0-9\.]+'), '-');
final file = File(p.join('.dart_tool', 'pub-search-data', fileName));
if (await file.exists()) {
var canUseCached = true;
if (ttl != null) {
final age = clock.now().difference(await file.lastModified());
if (age > ttl) {
canUseCached = false;
}
}
if (canUseCached) {
return await file.readAsString();
}
}

final content = await httpGetWithRetry(uri, responseFn: (rs) => rs.body);
await file.parent.create(recursive: true);
await file.writeAsString(content);
return content;
}

class _CombinedSearchIndex implements SearchIndex {
const _CombinedSearchIndex();

Expand Down
3 changes: 1 addition & 2 deletions app/lib/search/dart_sdk_mem_index.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ SdkMemIndex? get dartSdkMemIndex =>
Future<SdkMemIndex?> createDartSdkMemIndex() async {
try {
final index = await SdkMemIndex.dart();
final content =
await searchBackend.loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
final content = await loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
await index.addDartdocIndex(DartdocIndex.parseJsonText(content));
index.updateWeights(
libraryWeights: dartSdkLibraryWeights,
Expand Down
3 changes: 1 addition & 2 deletions app/lib/search/flutter_sdk_mem_index.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ SdkMemIndex? get flutterSdkMemIndex =>
Future<SdkMemIndex?> createFlutterSdkMemIndex() async {
try {
final index = SdkMemIndex.flutter();
final content =
await searchBackend.loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
final content = await loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
await index.addDartdocIndex(DartdocIndex.parseJsonText(content),
allowedLibraries: _allowedLibraries);
index.updateWeights(
Expand Down
3 changes: 1 addition & 2 deletions app/test/search/backend_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ void main() {
group('search backend', () {
testWithProfile('fetch SDK library description', fn: () async {
final index = await SdkMemIndex.dart();
final content = await searchBackend
.loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
final content = await loadOrFetchSdkIndexJsonAsString(index.indexJsonUri);
await index.addDartdocIndex(DartdocIndex.parseJsonText(content));
expect(
index.getLibraryDescription('dart:async'),
Expand Down
4 changes: 2 additions & 2 deletions app/test/search/dartdoc_index_parsing_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import '../shared/test_services.dart';
void main() {
group('dartdoc index.json parsing', () {
testWithProfile('parse Dart SDK index.json', fn: () async {
final textContent = await searchBackend.loadOrFetchSdkIndexJsonAsString(
final textContent = await loadOrFetchSdkIndexJsonAsString(
Uri.parse('https://api.dart.dev/stable/latest/index.json'),
ttl: Duration(days: 1),
);
Expand All @@ -35,7 +35,7 @@ void main() {
});

testWithProfile('parse Flutter SDK index.json', fn: () async {
final textContent = await searchBackend.loadOrFetchSdkIndexJsonAsString(
final textContent = await loadOrFetchSdkIndexJsonAsString(
Uri.parse('https://api.flutter.dev/flutter/index.json'),
ttl: Duration(days: 1),
);
Expand Down
44 changes: 44 additions & 0 deletions pkg/_pub_shared/lib/utils/http.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
// for details. All rights reserved. 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:io';

import 'package:http/http.dart' as http;
import 'package:http/retry.dart';
import 'package:retry/retry.dart';

final _transientStatusCodes = {
// See: https://cloud.google.com/storage/docs/xml-api/reference-status
Expand Down Expand Up @@ -35,3 +37,45 @@ http.Client httpRetryClient({
whenError: (e, st) => lenient || e is SocketException,
);
}

/// Creates a HTTP client and executes a GET request to the specified [uri],
/// making sure that the HTTP resources are freed after the [responseFn]
/// callback finishes.
/// The HTTP GET and the [responseFn] callback is retried on the transient
/// network errors.
Future<K> httpGetWithRetry<K>(
Uri uri, {
required FutureOr<K> Function(http.Response response) responseFn,
int maxAttempts = 3,
}) async {
return await retry(
() async {
final client = http.Client();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we pass in a client instead of creating a new client for every url to fetch?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Planning to do that in subsequent PRs (when migrating more functions to use this).

The practical difference in this specific case was negligible: we would only call the method when the index.json is not fetched at container creation time (or in local tests).

try {
final rs = await client.get(uri);
if (rs.statusCode == 200) {
return responseFn(rs);
}
throw http.ClientException(
'Unexpected status code for $uri: ${rs.statusCode}.');
} finally {
client.close();
}
},
maxAttempts: maxAttempts,
retryIf: _retryIf,
);
}

bool _retryIf(Exception e) {
if (e is TimeoutException) {
return true; // Timeouts we can retry
}
if (e is IOException) {
return true; // I/O issues are worth retrying
}
if (e is http.ClientException) {
return true; // HTTP issues are worth retrying
}
return false;
}
1 change: 1 addition & 0 deletions pkg/_pub_shared/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ dependencies:
logging: ^1.2.0
meta: ^1.3.0
pub_semver: ^2.0.0
retry: ^3.1.2
sanitize_html: ^2.1.0
api_builder:

Expand Down