Skip to content

Switch isDevToolsServerAvailable to not depend on the SSE connection #9024

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.

Sign up for GitHub

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
DanTup merged 2 commits into from
Mar 12, 2025
Merged

Switch isDevToolsServerAvailable to not depend on the SSE connection #9024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c731ea
Switch isDevToolsServerAvailable to not depend on the SSE connection
DanTup Mar 12, 2025
4fafb72
Remove unused import
DanTup Mar 12, 2025
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
35 changes: 23 additions & 12 deletions ...ls_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:js_interop';

import 'package:devtools_app_shared/utils.dart';
import 'package:devtools_app_shared/web_utils.dart';
import 'package:logging/logging.dart';
import 'package:web/web.dart' hide Storage;

import '../../../service/service_manager.dart';
Expand All @@ -14,6 +15,8 @@ import '../../primitives/storage.dart';
import '../../server/server.dart' as server;
import '../../server/server_api_client.dart';

final _log = Logger('framework_initialize');

/// Return the url the application is launched from.
Future<String> initializePlatform() async {
// Clear out the unneeded HTML from index.html.
Expand All @@ -27,20 +30,28 @@ Future<String> initializePlatform() async {
}.toJS,
);

// TODO(kenz): this server connection initialized listeners that are never
// disposed, so this is likely leaking resources.
// Here, we try and initialize the connection between the DevTools web app and
// its local server. DevTools can be launched without the server however, so
// establishing this connection is a best-effort.
// TODO(kenz): investigate if we can remove the DevToolsServerConnection
// code in general - it is currently only used for non-embedded pages to
// support some functionality like having VS Code reuse existing browser tabs
// and showing notifications if you try to launch when you already have one
// open.
final connection = await DevToolsServerConnection.connect();
if (connection != null) {
// Check if the server API is available.
if (await server.checkServerHttpApiAvailable()) {
_log.info('Server HTTP API is available, using server for storage.');
setGlobal(Storage, server.ServerConnectionStorage());

// And also connect the legacy SSE API if necessary
// (`DevToolsServerConnection.connect`) may short-circuit in some cases,
// such as when embedded.

// TODO(kenz): this server connection initialized listeners that are never
// disposed, so this is likely leaking resources.
// Here, we try and initialize the connection between the DevTools web app and
// its local server. DevTools can be launched without the server however, so
// establishing this connection is a best-effort.
// TODO(kenz): investigate if we can remove the DevToolsServerConnection
// code in general - it is currently only used for non-embedded pages to
// support some functionality like having VS Code reuse existing browser tabs
// and showing notifications if you try to launch when you already have one
// open.
await DevToolsServerConnection.connect();
} else {
_log.info('Server HTTP API is not available, using browser for storage.');
setGlobal(Storage, BrowserStorage());
}

Expand Down
31 changes: 31 additions & 0 deletions packages/devtools_app/lib/src/shared/server/_server_api.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2025 The Flutter Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.

part of 'server.dart';

/// Checks whether the server HTTP API is available.
Future<bool> checkServerHttpApiAvailable() async {
try {
// Unlike other parts of this API, the ping request is handled directly
// in the server in the SDK (see `pkg\dds\lib\src\devtools\handler.dart`)
// and not delegated back to DevTools shared code.
final response = await get(
Uri.parse('${apiPrefix}ping'),
).timeout(const Duration(seconds: 5));
// When running with the local dev server Flutter may serve its index page
// for missing files to support the hashless url strategy. Check the response
// content to confirm it came from our server.
// See https://github.com/flutter/flutter/issues/67053
if (response.statusCode != 200 || response.body != 'OK') {
_log.info('DevTools server not available (${response.statusCode})');
return false;
}
} catch (e) {
// unable to locate dev server
_log.info('DevTools server not available ($e)');
return false;
}

return true;
}
9 changes: 8 additions & 1 deletion packages/devtools_app/lib/src/shared/server/server.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,22 @@ part '_dtd_api.dart';
part '_extensions_api.dart';
part '_preferences_api.dart';
part '_release_notes_api.dart';
part '_server_api.dart';
part '_survey_api.dart';

final _log = Logger('devtools_server_client');

/// Whether the DevTools server is available.
/// Whether the DevTools server is available so that the HTTP API can be used.
///
/// A value of `true` here does not necessarily mean the legacy SSE API is
/// available.
///
/// Since the DevTools server is a web server, it is only available for the
/// web platform.
///
/// TODO(dantup): Since this relates only to non-SSE API, it could be available
/// for non-web?
Comment on lines 38 to +42
Copy link
Contributor Author

@DanTup DanTup Mar 12, 2025

Choose a reason for hiding this comment

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

I'm not sure of the logic behind this comment, but wonder if maybe it was because of SSE (and maybe package:sse only worked for web?). I feel like we should (in theory) be able to run the server and connect a desktop version of DevTools to it, which makes this comment incorrect?

///
/// In `framework_initialize_web.dart`, we test the DevTools server connection
/// by pinging the server and checking the response. If this is successful, we
/// set the [storage] global to an instance of [ServerConnectionStorage].
Expand Down
27 changes: 5 additions & 22 deletions packages/devtools_app/lib/src/shared/server/server_api_client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'dart:convert';
import 'package:devtools_app_shared/ui.dart' show isEmbedded;
import 'package:devtools_shared/devtools_shared.dart';
import 'package:flutter/foundation.dart';
import 'package:http/http.dart' as http;
import 'package:logging/logging.dart';

import '../config_specific/notifications/notifications.dart';
Expand Down Expand Up @@ -47,6 +46,11 @@ class DevToolsServerConnection {
? baseUri.resolve('devtools/api/')
: baseUri.resolve('api/');

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

final serverUri = Uri.parse(devToolsServerUriAsString);
final apiUri = apiUriFor(serverUri);
final pingUri = apiUri.resolve('ping');

try {
final response = await http
.get(pingUri)
.timeout(const Duration(seconds: 5));
// When running with the local dev server Flutter may serve its index page
// for missing files to support the hashless url strategy. Check the response
// content to confirm it came from our server.
// See https://github.com/flutter/flutter/issues/67053
if (response.statusCode != 200 || response.body != 'OK') {
// unable to locate dev server
_log.info('devtools server not available (${response.statusCode})');
return null;
}
} catch (e) {
// unable to locate dev server
_log.info('devtools server not available ($e)');
return null;
}

final sseUri = apiUri.resolve('sse');
final client = SseClient(sseUri.toString(), debugKey: 'DevToolsServer');
return DevToolsServerConnection._(client);
Expand Down