Skip to content

Commit

Permalink
Fetch log details immediately upon receiving logs (#8421)
Browse files Browse the repository at this point in the history
  • Loading branch information
kenzieschmoll authored Oct 9, 2024
1 parent 551ebc8 commit 04ce313
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 36 deletions.
49 changes: 38 additions & 11 deletions packages/devtools_app/lib/src/screens/logging/_message_column.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ class MessageColumn extends ColumnData<LogData>
bool isRowHovered = false,
VoidCallback? onPressed,
}) {
final theme = Theme.of(context);
final hasSummary = !data.summary.isNullOrEmpty;
// This needs to be a function because the details may be computed after the
// initial build.
bool hasDetails() => !data.details.isNullOrEmpty;

if (data.kind.caseInsensitiveEquals(FlutterEvent.frame)) {
const color = Color.fromARGB(0xff, 0x00, 0x91, 0xea);
final text = Text(
Expand Down Expand Up @@ -81,17 +87,38 @@ class MessageColumn extends ColumnData<LogData>
],
);
} else {
return RichText(
text: TextSpan(
children: processAnsiTerminalCodes(
// TODO(helin24): Recompute summary length considering ansi codes.
// The current summary is generally the first 200 chars of details.
getDisplayValue(data),
Theme.of(context).regularTextStyle,
),
),
overflow: TextOverflow.ellipsis,
maxLines: 1,
return ValueListenableBuilder<bool>(
valueListenable: data.detailsComputed,
builder: (context, detailsComputed, _) {
return RichText(
text: TextSpan(
children: [
if (hasSummary)
...processAnsiTerminalCodes(
// TODO(helin24): Recompute summary length considering ansi codes.
// The current summary is generally the first 200 chars of details.
data.summary!,
theme.regularTextStyle,
),
if (hasSummary && hasDetails())
WidgetSpan(
child: Icon(
Icons.arrow_right,
size: defaultIconSize,
color: theme.colorScheme.onSurface,
),
),
if (hasDetails())
...processAnsiTerminalCodes(
detailsComputed ? data.details! : '<fetching>',
theme.subtleTextStyle,
),
],
),
overflow: TextOverflow.ellipsis,
maxLines: 1,
);
},
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import 'package:logging/logging.dart';
import 'package:path/path.dart' as path;
import 'package:vm_service/vm_service.dart';

import '../../framework/app_error_handling.dart' as error_handling;
import '../../service/vm_service_wrapper.dart';
import '../../shared/diagnostics/diagnostics_node.dart';
import '../../shared/diagnostics/inspector_service.dart';
Expand Down Expand Up @@ -379,7 +380,8 @@ class LoggingController extends DisposableController
final error = InstanceRef.parse(logRecord.error);
final stackTrace = InstanceRef.parse(logRecord.stackTrace);

final details = summary;
// TODO(kenz): we may want to narrow down the details of dart developer logs
final details = jsonEncode(e.json);
Future<String> Function()? detailsComputer;

// If the message string was truncated by the VM, or the error object or
Expand Down Expand Up @@ -741,7 +743,23 @@ class LogData with SearchableDataMixin {
this.isError = false,
this.detailsComputer,
this.node,
});
}) {
final originalDetails = _details;
// Fetch details immediately on creation.
unawaited(
compute().catchError(
(Object? error) {
// On error, set the value of details to its original value.
_details = originalDetails;
_detailsComputed.value = true;
error_handling.reportError(
'Error fetching details for $kind log'
'${error != null ? ': $error' : ''}.',
);
},
),
);
}

final String kind;
final int? timestamp;
Expand All @@ -756,24 +774,33 @@ class LogData with SearchableDataMixin {

String? get details => _details;

bool get needsComputing => detailsComputer != null;
bool get needsComputing => !detailsComputed.value;

ValueListenable<bool> get detailsComputed => _detailsComputed;
final _detailsComputed = ValueNotifier<bool>(false);

Future<void> compute() async {
_details = await detailsComputer!();
detailsComputer = null;
if (!detailsComputed.value) {
if (detailsComputer != null) {
_details = await detailsComputer!();
}
detailsComputer = null;
_detailsComputed.value = true;
}
}

String? prettyPrinted() {
if (needsComputing) {
return details;
if (!detailsComputed.value) {
return details?.trim();
}

try {
return prettyPrinter
.convert(jsonDecode(details!))
.replaceAll(r'\n', '\n');
.replaceAll(r'\n', '\n')
.trim();
} catch (_) {
return details;
return details?.trim();
}
}

Expand Down
12 changes: 2 additions & 10 deletions packages/devtools_app/lib/src/screens/logging/logging_screen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ class _LoggingScreenState extends State<LoggingScreenBody>
with
AutoDisposeMixin,
ProvidedControllerMixin<LoggingController, LoggingScreenBody> {
late List<LogData> filteredLogs;

@override
void initState() {
super.initState();
Expand All @@ -87,13 +85,7 @@ class _LoggingScreenState extends State<LoggingScreenBody>
if (!initController()) return;

cancelListeners();

filteredLogs = controller.filteredData.value;
addAutoDisposeListener(controller.filteredData, () {
setState(() {
filteredLogs = controller.filteredData.value;
});
});
addAutoDisposeListener(controller.filteredData);
}

@override
Expand Down Expand Up @@ -172,7 +164,7 @@ class _LoggingScreenState extends State<LoggingScreenBody>
RoundedOutlinedBorder(
clip: true,
child: LogsTable(
data: filteredLogs,
data: controller.filteredData.value,
selectionNotifier: controller.selectedLog,
searchMatchesNotifier: controller.searchMatches,
activeSearchMatchNotifier: controller.activeSearchMatch,
Expand Down
3 changes: 3 additions & 0 deletions packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ TODO: Remove this section if there are not any general updates.

## Logging updates

* Fetch log details immediately upon receiving logs so that log data is not lost
due to lazy loading. - [#8421](https://github.com/flutter/devtools/pull/8421)

* Fix a bug where logs would get out of order after midnight. -
[#8420](https://github.com/flutter/devtools/pull/8420)

Expand Down
23 changes: 17 additions & 6 deletions packages/devtools_app/test/logging/logging_screen_data_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ void main() {
}

setUp(() {
// Reset the log data for each test so that the delay for computing the
// details behaves the same for each test.
_fakeLogData = null;

mockLoggingController =
createMockLoggingControllerWithDefaults(data: fakeLogData);

Expand Down Expand Up @@ -77,16 +81,21 @@ void main() {
await tester.pumpAndSettle();
await tester.tap(find.byKey(ValueKey(fakeLogData[6])));
await tester.pumpAndSettle();

expect(
find.selectableText('log event 6'),
find.descendant(
of: find.byType(LogsTable),
matching: find.richTextContaining('log event 6'),
),
findsOneWidget,
reason:
'The log details should be visible both in the details section.',
);
expect(
find.selectableText('log event 6'),
find.descendant(
of: find.byType(LogDetails),
matching: find.selectableText('log event 6'),
),
findsOneWidget,
reason: 'The log details should be visible both in the table.',
reason: 'The log details should now be visible in the details section.',
);
},
);
Expand Down Expand Up @@ -219,7 +228,9 @@ void main() {

const totalLogs = 10;

final fakeLogData = List<LogData>.generate(totalLogs, _generate);
List<LogData> get fakeLogData =>
_fakeLogData ??= List<LogData>.generate(totalLogs, _generate);
List<LogData>? _fakeLogData;

LogData _generate(int i) {
String? details = 'log event $i';
Expand Down

0 comments on commit 04ce313

Please sign in to comment.