Skip to content

Commit c4bd441

Browse files
authored
Switch isDevToolsServerAvailable to not depend on the SSE connection (#9024)
* Switch isDevToolsServerAvailable to not depend on the SSE connection My recent change to disable the SSE connection when running embedded had an unintended side-effect. There's a number of places that check `isDevToolsServerAvailable` to see if they can call the HTTP API (and to set up server-side storage), however this was checking whether the SSE connection was made, not only that the server was available. This change moves the server API check out of `DevToolsServerConnection` (which is basically the SSE API) and sets the storage (and therefore `isDevToolsServerAvailable`) based on just the HTTP API regardless of SSE, and then only attempts to connect SSE after. Fixes #9023 * Remove unused import
1 parent cfff32d commit c4bd441

4 files changed

Lines changed: 67 additions & 35 deletions

File tree

packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:js_interop';
66

77
import 'package:devtools_app_shared/utils.dart';
88
import 'package:devtools_app_shared/web_utils.dart';
9+
import 'package:logging/logging.dart';
910
import 'package:web/web.dart' hide Storage;
1011

1112
import '../../../service/service_manager.dart';
@@ -14,6 +15,8 @@ import '../../primitives/storage.dart';
1415
import '../../server/server.dart' as server;
1516
import '../../server/server_api_client.dart';
1617

18+
final _log = Logger('framework_initialize');
19+
1720
/// Return the url the application is launched from.
1821
Future<String> initializePlatform() async {
1922
// Clear out the unneeded HTML from index.html.
@@ -27,20 +30,28 @@ Future<String> initializePlatform() async {
2730
}.toJS,
2831
);
2932

30-
// TODO(kenz): this server connection initialized listeners that are never
31-
// disposed, so this is likely leaking resources.
32-
// Here, we try and initialize the connection between the DevTools web app and
33-
// its local server. DevTools can be launched without the server however, so
34-
// establishing this connection is a best-effort.
35-
// TODO(kenz): investigate if we can remove the DevToolsServerConnection
36-
// code in general - it is currently only used for non-embedded pages to
37-
// support some functionality like having VS Code reuse existing browser tabs
38-
// and showing notifications if you try to launch when you already have one
39-
// open.
40-
final connection = await DevToolsServerConnection.connect();
41-
if (connection != null) {
33+
// Check if the server API is available.
34+
if (await server.checkServerHttpApiAvailable()) {
35+
_log.info('Server HTTP API is available, using server for storage.');
4236
setGlobal(Storage, server.ServerConnectionStorage());
37+
38+
// And also connect the legacy SSE API if necessary
39+
// (`DevToolsServerConnection.connect`) may short-circuit in some cases,
40+
// such as when embedded.
41+
42+
// TODO(kenz): this server connection initialized listeners that are never
43+
// disposed, so this is likely leaking resources.
44+
// Here, we try and initialize the connection between the DevTools web app and
45+
// its local server. DevTools can be launched without the server however, so
46+
// establishing this connection is a best-effort.
47+
// TODO(kenz): investigate if we can remove the DevToolsServerConnection
48+
// code in general - it is currently only used for non-embedded pages to
49+
// support some functionality like having VS Code reuse existing browser tabs
50+
// and showing notifications if you try to launch when you already have one
51+
// open.
52+
await DevToolsServerConnection.connect();
4353
} else {
54+
_log.info('Server HTTP API is not available, using browser for storage.');
4455
setGlobal(Storage, BrowserStorage());
4556
}
4657

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2025 The Flutter Authors
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
4+
5+
part of 'server.dart';
6+
7+
/// Checks whether the server HTTP API is available.
8+
Future<bool> checkServerHttpApiAvailable() async {
9+
try {
10+
// Unlike other parts of this API, the ping request is handled directly
11+
// in the server in the SDK (see `pkg\dds\lib\src\devtools\handler.dart`)
12+
// and not delegated back to DevTools shared code.
13+
final response = await get(
14+
Uri.parse('${apiPrefix}ping'),
15+
).timeout(const Duration(seconds: 5));
16+
// When running with the local dev server Flutter may serve its index page
17+
// for missing files to support the hashless url strategy. Check the response
18+
// content to confirm it came from our server.
19+
// See https://github.com/flutter/flutter/issues/67053
20+
if (response.statusCode != 200 || response.body != 'OK') {
21+
_log.info('DevTools server not available (${response.statusCode})');
22+
return false;
23+
}
24+
} catch (e) {
25+
// unable to locate dev server
26+
_log.info('DevTools server not available ($e)');
27+
return false;
28+
}
29+
30+
return true;
31+
}

packages/devtools_app/lib/src/shared/server/server.dart

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,22 @@ part '_dtd_api.dart';
2525
part '_extensions_api.dart';
2626
part '_preferences_api.dart';
2727
part '_release_notes_api.dart';
28+
part '_server_api.dart';
2829
part '_survey_api.dart';
2930

3031
final _log = Logger('devtools_server_client');
3132

32-
/// Whether the DevTools server is available.
33+
/// Whether the DevTools server is available so that the HTTP API can be used.
34+
///
35+
/// A value of `true` here does not necessarily mean the legacy SSE API is
36+
/// available.
3337
///
3438
/// Since the DevTools server is a web server, it is only available for the
3539
/// web platform.
3640
///
41+
/// TODO(dantup): Since this relates only to non-SSE API, it could be available
42+
/// for non-web?
43+
///
3744
/// In `framework_initialize_web.dart`, we test the DevTools server connection
3845
/// by pinging the server and checking the response. If this is successful, we
3946
/// set the [storage] global to an instance of [ServerConnectionStorage].

packages/devtools_app/lib/src/shared/server/server_api_client.dart

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import 'dart:convert';
88
import 'package:devtools_app_shared/ui.dart' show isEmbedded;
99
import 'package:devtools_shared/devtools_shared.dart';
1010
import 'package:flutter/foundation.dart';
11-
import 'package:http/http.dart' as http;
1211
import 'package:logging/logging.dart';
1312

1413
import '../config_specific/notifications/notifications.dart';
@@ -47,6 +46,11 @@ class DevToolsServerConnection {
4746
? baseUri.resolve('devtools/api/')
4847
: baseUri.resolve('api/');
4948

49+
/// Connects to the legacy SSE API.
50+
///
51+
/// Callers should first ensure the DevTools server is available (for example
52+
/// by calling [checkServerHttpApiAvailable] or verifying that it was
53+
/// successfull by using [isDevToolsServerAvailable])
5054
static Future<DevToolsServerConnection?> connect() async {
5155
// Don't connect SSE when running embedded because the API does not provide
5256
// anything that is used when embedded but it ties up one of the limited
@@ -58,27 +62,6 @@ class DevToolsServerConnection {
5862

5963
final serverUri = Uri.parse(devToolsServerUriAsString);
6064
final apiUri = apiUriFor(serverUri);
61-
final pingUri = apiUri.resolve('ping');
62-
63-
try {
64-
final response = await http
65-
.get(pingUri)
66-
.timeout(const Duration(seconds: 5));
67-
// When running with the local dev server Flutter may serve its index page
68-
// for missing files to support the hashless url strategy. Check the response
69-
// content to confirm it came from our server.
70-
// See https://github.com/flutter/flutter/issues/67053
71-
if (response.statusCode != 200 || response.body != 'OK') {
72-
// unable to locate dev server
73-
_log.info('devtools server not available (${response.statusCode})');
74-
return null;
75-
}
76-
} catch (e) {
77-
// unable to locate dev server
78-
_log.info('devtools server not available ($e)');
79-
return null;
80-
}
81-
8265
final sseUri = apiUri.resolve('sse');
8366
final client = SseClient(sseUri.toString(), debugKey: 'DevToolsServer');
8467
return DevToolsServerConnection._(client);

0 commit comments

Comments
 (0)