diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c5ec45..16d7ea8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ board can be cross-checked against what actually shipped. ## [Unreleased] +### Fixed + +- A call whose answer has no body stayed pending, with a duration that kept growing, when the app never read the answer, as it does not after looking at the status of a conditional request. The call now ends when the head arrives, for the answer to a HEAD request, a 204, a 304 and an answer that says it is empty. A redirect still waits, because the client may follow it +- The recording dot, and the status and the time of a call that is still running, pulsed for as long as they were on screen, which kept the app drawing a frame on every refresh with nothing else changing. A pulse now beats three times when it appears, the recording dot beats again when the history changes, and then it holds still. It also no longer starts when the system asks for reduced motion, where it ran without being seen + ### Internal - The release workflow looks up the archive of each package by the version of the tag, for the attestation and for the record on the Packages panel. It used the latest version that pub.dev lists, which can still be the previous one for minutes after an upload, and on 1.0.0 it was for two of the four packages when the record job read it diff --git a/packages/flutwire/lib/src/capture/recorder.dart b/packages/flutwire/lib/src/capture/recorder.dart index be048db..a66bfdd 100644 --- a/packages/flutwire/lib/src/capture/recorder.dart +++ b/packages/flutwire/lib/src/capture/recorder.dart @@ -102,6 +102,13 @@ final class FlutwireCallRecorder { /// Reports the response head. Calling it a second time, as a client does /// after following a redirect, replaces the first. + /// + /// A response that has no body ends the call here: the answer to a HEAD + /// request, a 204, a 304 and one that says it is empty. A client reads such a + /// body only when it wants to, and most never do, so waiting for the stream + /// to end would leave the call running for as long as the app does. A + /// redirect is left open, because the client may follow it and report the + /// head of what it finds. void responseHead({ required int statusCode, required FlutwireHeaders headers, @@ -137,7 +144,11 @@ final class FlutwireCallRecorder { compression: compression, ), ); - _publish(finished: false); + if (_endsAtHead(statusCode, headers)) { + _end(FlutwireCallState.completed); + } else { + _publish(finished: false); + } }); /// Adds a chunk of the response body. @@ -284,6 +295,22 @@ final class FlutwireCallRecorder { static const Duration _progressInterval = Duration(milliseconds: 200); + static const Set _redirectStatuses = {301, 302, 303, 307, 308}; + + /// Whether nothing can follow the head of this response, so that the call is + /// over with it. RFC 9110, section 6.4.1, says which responses have no body. + bool _endsAtHead(int statusCode, FlutwireHeaders headers) { + // After a protocol switch come the bytes of the new protocol, and a client + // may follow a redirect and report the head of what it finds. + if (statusCode < 200 || _redirectStatuses.contains(statusCode)) { + return false; + } + return _call.method == 'HEAD' || + statusCode == 204 || + statusCode == 304 || + int.tryParse(headers.first('content-length')?.trim() ?? '') == 0; + } + /// The address of a WebSocket as the app wrote it. The handshake is an HTTP /// request, so the capture is handed an `http` or `https` address, with a /// port of 0 when the app gave none and an empty fragment. diff --git a/packages/flutwire/lib/src/ui/inspector/recording_row.dart b/packages/flutwire/lib/src/ui/inspector/recording_row.dart index d2bc237..be96dd6 100644 --- a/packages/flutwire/lib/src/ui/inspector/recording_row.dart +++ b/packages/flutwire/lib/src/ui/inspector/recording_row.dart @@ -46,6 +46,7 @@ class RecordingRow extends StatelessWidget { children: [ Pulse( active: !paused, + restartOn: session.store.calls, child: Container( width: 8, height: 8, diff --git a/packages/flutwire/lib/src/ui/widgets/pulse.dart b/packages/flutwire/lib/src/ui/widgets/pulse.dart index b03e820..d2ff7cb 100644 --- a/packages/flutwire/lib/src/ui/widgets/pulse.dart +++ b/packages/flutwire/lib/src/ui/widgets/pulse.dart @@ -1,10 +1,21 @@ import 'package:flutter/material.dart'; -/// Fades its child in and out, slowly, to say that something is in progress. -/// It holds still when the system asks for reduced motion. +/// Fades its child out and in a few times, to say that something is going on, +/// and then holds still. +/// +/// It beats when it first appears active, when it becomes active, and when +/// [restartOn] notifies after the beats are over. It never goes on until told +/// to stop: an animation that runs for as long as the inspector is open keeps +/// the app drawing a frame on every refresh for nothing. It holds still as well +/// when the system asks for reduced motion. class Pulse extends StatefulWidget { /// Pulses [child]. - const Pulse({super.key, required this.child, this.active = true}); + const Pulse({ + super.key, + required this.child, + this.active = true, + this.restartOn, + }); /// What pulses. final Widget child; @@ -12,50 +23,106 @@ class Pulse extends StatefulWidget { /// Whether it pulses at all. final bool active; + /// Makes it beat again each time it notifies, as new traffic does. + final Listenable? restartOn; + @override State createState() => _PulseState(); } class _PulseState extends State with SingleTickerProviderStateMixin { + static const int _beats = 3; + static const Duration _beat = Duration(milliseconds: 2200); + + /// Out to a dim and back in, once for each beat, ending as it began. + static final Animatable _fade = TweenSequence([ + for (var i = 0; i < _beats; i++) ...[ + _half(from: 1, to: 0.4), + _half(from: 0.4, to: 1), + ], + ]); + + static TweenSequenceItem _half({ + required double from, + required double to, + }) => TweenSequenceItem( + tween: Tween( + begin: from, + end: to, + ).chain(CurveTween(curve: Curves.easeInOut)), + weight: 1, + ); + late final AnimationController _controller = AnimationController( vsync: this, - duration: const Duration(milliseconds: 1100), + duration: _beat * _beats, ); + late final Animation _opacity = _controller.drive(_fade); + + bool _reduced = false; + + /// Whether the beats for the current activation have been started. + bool _started = false; @override void initState() { super.initState(); - if (widget.active) _controller.repeat(reverse: true); + widget.restartOn?.addListener(_restart); + } + + @override + void didChangeDependencies() { + super.didChangeDependencies(); + _reduced = MediaQuery.disableAnimationsOf(context); + if (_reduced) { + _hold(); + } else if (widget.active && !_started) { + _begin(); + } } @override void didUpdateWidget(Pulse old) { super.didUpdateWidget(old); - if (widget.active && !_controller.isAnimating) { - _controller.repeat(reverse: true); - } else if (!widget.active && _controller.isAnimating) { - _controller.stop(); - _controller.value = 0; + if (old.restartOn != widget.restartOn) { + old.restartOn?.removeListener(_restart); + widget.restartOn?.addListener(_restart); + } + if (!widget.active) { + if (old.active) _hold(); + } else if (!_started && !_reduced) { + _begin(); } } @override void dispose() { + widget.restartOn?.removeListener(_restart); _controller.dispose(); super.dispose(); } + void _begin() { + _started = true; + _controller.forward(from: 0); + } + + void _hold() { + _started = false; + _controller.stop(); + _controller.value = 0; + } + + /// Notifications come in bursts, and one call tells of itself several times + /// as it goes. Those that arrive while the beats play do not add to them. + void _restart() { + if (!widget.active || _reduced || _controller.isAnimating) return; + _begin(); + } + @override Widget build(BuildContext context) { - if (!widget.active || MediaQuery.disableAnimationsOf(context)) { - return widget.child; - } - return FadeTransition( - opacity: Tween( - begin: 1, - end: 0.4, - ).animate(CurvedAnimation(parent: _controller, curve: Curves.easeInOut)), - child: widget.child, - ); + if (!widget.active || _reduced) return widget.child; + return FadeTransition(opacity: _opacity, child: widget.child); } } diff --git a/packages/flutwire/test/capture/recorder_test.dart b/packages/flutwire/test/capture/recorder_test.dart index c6f5bcf..dbbf4be 100644 --- a/packages/flutwire/test/capture/recorder_test.dart +++ b/packages/flutwire/test/capture/recorder_test.dart @@ -132,6 +132,118 @@ void main() { }); }); + // A client reads the body of such a response only when it feels like it, and + // for most of them it never does. Waiting for a reader would leave the call + // running for as long as the app does. + group('a response that has no body', () { + FlutwireCall answer( + int status, { + String method = 'GET', + Map headers = const {}, + }) { + final t = make(); + final rec = t.engine.begin( + method: method, + uri: Uri.parse('https://x.test/'), + source: FlutwireSource.io, + )!; + rec.responseHead( + statusCode: status, + headers: FlutwireHeaders.fromMap(headers), + ); + return t.store.find(rec.id)!; + } + + test('ends the call at its head when it is a 304 or a 204', () { + for (final status in [204, 304]) { + final call = answer(status); + expect(call.state, FlutwireCallState.completed, reason: '$status'); + expect(call.statusCode, status); + expect(call.endedAt, isNotNull); + expect(call.response!.body.totalBytes, 0); + } + }); + + test('ends the call at its head when it answers a HEAD request', () { + expect(answer(200, method: 'HEAD').state, FlutwireCallState.completed); + }); + + test('ends the call at its head when it says it is empty', () { + final call = answer(201, headers: {'Content-Length': ' 0 '}); + expect(call.state, FlutwireCallState.completed); + }); + + test('leaves the call open when a body may follow', () { + expect(answer(200).state, FlutwireCallState.pending); + expect( + answer(200, headers: {'content-length': '12'}).state, + FlutwireCallState.pending, + ); + // The bytes of the protocol it switches to follow a 101, whatever else + // it says about its length. + expect( + answer(101, headers: {'upgrade': 'websocket'}).state, + FlutwireCallState.pending, + ); + expect( + answer( + 101, + headers: {'upgrade': 'websocket', 'content-length': '0'}, + ).state, + FlutwireCallState.pending, + ); + }); + + test('leaves a redirect open, because the client may follow it', () { + expect(answer(301, method: 'HEAD').state, FlutwireCallState.pending); + + final t = make(); + final rec = t.engine.begin( + method: 'GET', + uri: Uri.parse('https://x.test/'), + source: FlutwireSource.io, + )!; + rec.responseHead( + statusCode: 302, + headers: FlutwireHeaders.fromMap({ + 'content-length': '0', + 'location': '/next', + }), + ); + expect(t.store.find(rec.id)!.state, FlutwireCallState.pending); + + rec + ..responseHead( + statusCode: 200, + headers: FlutwireHeaders.fromMap({'content-type': 'text/plain'}), + ) + ..responseBody(utf('hello')) + ..complete(); + final call = t.store.find(rec.id)!; + expect(call.statusCode, 200); + expect(call.response!.body.text, 'hello'); + }); + + test('ignores the reports that come after the end', () { + final t = make(); + final rec = t.engine.begin( + method: 'GET', + uri: Uri.parse('https://x.test/'), + source: FlutwireSource.io, + )!; + rec.responseHead(statusCode: 304, headers: FlutwireHeaders.empty); + final ended = t.store.find(rec.id)!.endedAt; + t.clock.advance(const Duration(seconds: 5)); + rec + ..responseBody(utf('late')) + ..cancel(); + final call = t.store.find(rec.id)!; + expect(call.state, FlutwireCallState.completed); + expect(call.endedAt, ended); + expect(call.response!.body.totalBytes, 0); + }); + }); + test('a response is hidden by the redaction policy, body included', () { final t = make( const FlutwireConfig( diff --git a/packages/flutwire/test/capture/wire_io_test.dart b/packages/flutwire/test/capture/wire_io_test.dart index 9163ddc..499d1c9 100644 --- a/packages/flutwire/test/capture/wire_io_test.dart +++ b/packages/flutwire/test/capture/wire_io_test.dart @@ -45,6 +45,12 @@ Future handle(HttpRequest req) async { } else if (path.startsWith('/status/')) { req.response.statusCode = int.parse(path.substring(8)); req.response.write('nope'); + } else if (path == '/not-modified') { + req.response.statusCode = HttpStatus.notModified; + } else if (path == '/no-content') { + req.response.statusCode = HttpStatus.noContent; + } else if (path == '/empty') { + req.response.contentLength = 0; } else if (path == '/big') { for (var i = 0; i < 100; i++) { req.response.add(List.filled(4096, 65)); @@ -318,6 +324,70 @@ void main() { final call = (await settled()).single; expect(call.state, FlutwireCallState.cancelled); }); + + // The app often never reads these: it looks at the status of a conditional + // request and goes on, so nothing listens to the stream, and a call that + // ended with its stream would stay open for as long as the app runs. + group('an answer with no body', () { + for (final (name, path, status) in [ + ('a 304', '/not-modified', 304), + ('a 204', '/no-content', 204), + ('a 200 that says it is empty', '/empty', 200), + ]) { + test('$name that the app never reads is a completed call', () async { + start(); + final client = HttpClient(); + final request = await client.getUrl(Uri.parse('$base$path')); + final response = await request.close(); + expect(response.statusCode, status); + + final call = (await settled()).single; + client.close(); + expect(call.state, FlutwireCallState.completed); + expect(call.statusCode, status); + expect(call.endedAt, isNotNull); + expect(call.response!.body.totalBytes, 0); + }); + } + + test('the answer to a HEAD request is a completed call', () async { + start(); + final client = HttpClient(); + final request = await client.headUrl(Uri.parse('$base/json')); + final response = await request.close(); + expect(response.statusCode, 200); + + final call = (await settled()).single; + client.close(); + expect(call.method, 'HEAD'); + expect(call.state, FlutwireCallState.completed); + }); + + test('package:http records a 304 whose stream is never read', () async { + start(); + final client = http.Client(); + final response = await client.send( + http.Request('GET', Uri.parse('$base/not-modified')), + ); + expect(response.statusCode, 304); + + final call = (await settled()).single; + client.close(); + expect(call.state, FlutwireCallState.completed); + expect(call.statusCode, 304); + }); + + test('a body that has not been read yet keeps the call open', () async { + start(); + final client = HttpClient(); + final request = await client.getUrl(Uri.parse('$base/big')); + await request.close(); + + final call = (await settled()).single; + client.close(force: true); + expect(call.state, FlutwireCallState.pending); + }); + }); } class _CountingOverrides extends HttpOverrides { diff --git a/packages/flutwire/test/ui/calls.dart b/packages/flutwire/test/ui/calls.dart index f25be27..185954f 100644 --- a/packages/flutwire/test/ui/calls.dart +++ b/packages/flutwire/test/ui/calls.dart @@ -72,8 +72,9 @@ Widget inspectorApp(FlutwireSession session, Widget home) => MaterialApp( home: FlutwireShell(session: session, child: home), ); -/// The inspector pulses while it records, so `pumpAndSettle` never settles: -/// this lets the animations and the short timers run instead. +/// The inspector pulses when it opens and whenever a call arrives, so +/// `pumpAndSettle` would also run those beats out: this lets the animations and +/// the short timers run for a fixed time instead. Future settle(WidgetTester tester, [int milliseconds = 500]) async { await tester.pump(); await tester.pump(Duration(milliseconds: milliseconds)); diff --git a/packages/flutwire/test/ui/inspector_page_test.dart b/packages/flutwire/test/ui/inspector_page_test.dart index 181e20c..1f8090d 100644 --- a/packages/flutwire/test/ui/inspector_page_test.dart +++ b/packages/flutwire/test/ui/inspector_page_test.dart @@ -118,6 +118,71 @@ void main() { }); }); + // The dot pulses while traffic flows and rests when none does. One that + // never rests keeps the app drawing a frame on every refresh for as long as + // the inspector is open, which on a phone is heat and battery for nothing. + group('the pulse', () { + const longerThanTheBeats = Duration(seconds: 12); + + testWidgets('rests once nothing is arriving', (tester) async { + await open(tester); + expect(tester.hasRunningAnimations, isTrue); + + await tester.pump(longerThanTheBeats); + + expect(tester.hasRunningAnimations, isFalse); + }); + + testWidgets('beats when a call arrives', (tester) async { + final session = await open(tester); + await tester.pump(longerThanTheBeats); + expect(tester.hasRunningAnimations, isFalse); + + session.store + ..put(makeCall(5, url: 'https://api.test/new')) + ..flush(); + await tester.pump(); + expect(tester.hasRunningAnimations, isTrue); + + await tester.pump(longerThanTheBeats); + expect(tester.hasRunningAnimations, isFalse); + }); + + testWidgets('does not beat while recording is paused', (tester) async { + final session = await open(tester); + session.paused.value = true; + await tester.pump(longerThanTheBeats); + + session.store + ..put(makeCall(5, url: 'https://api.test/new')) + ..flush(); + await tester.pump(); + + expect(tester.hasRunningAnimations, isFalse); + }); + + testWidgets('a call that is still running does not keep it going', ( + tester, + ) async { + await open( + tester, + calls: [ + makeCall( + 1, + url: 'https://api.test/stream', + status: null, + state: FlutwireCallState.pending, + ), + ], + ); + expect(find.text('ยทยทยท'), findsOneWidget); + + await tester.pump(longerThanTheBeats); + + expect(tester.hasRunningAnimations, isFalse); + }); + }); + group('search and filters', () { Future type(WidgetTester tester, String text) async { await tester.enterText(find.byType(TextField), text); diff --git a/packages/flutwire/test/ui/pulse_test.dart b/packages/flutwire/test/ui/pulse_test.dart new file mode 100644 index 0000000..6cec856 --- /dev/null +++ b/packages/flutwire/test/ui/pulse_test.dart @@ -0,0 +1,101 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutwire/src/ui/widgets/pulse.dart'; + +// A pulse that never stops keeps the app drawing a frame on every refresh for +// nothing, and the inspector stays open for a long time. These check that it +// beats for a while and then holds still, so that a screen at rest costs +// nothing. +const _longerThanTheBeats = Duration(seconds: 12); + +Widget _app(Widget child, {bool reducedMotion = false}) => MaterialApp( + builder: (context, home) => MediaQuery( + data: MediaQuery.of(context).copyWith(disableAnimations: reducedMotion), + child: home!, + ), + home: Scaffold(body: Center(child: child)), +); + +const _dot = SizedBox.square(dimension: 8); + +// The page transitions of the app have fades of their own. +Finder _fade() => find.descendant( + of: find.byType(Pulse), + matching: find.byType(FadeTransition), +); + +double _opacity(WidgetTester tester) => + tester.widget(_fade()).opacity.value; + +void main() { + testWidgets('beats when it appears and then holds still', (tester) async { + await tester.pumpWidget(_app(const Pulse(child: _dot))); + expect(tester.hasRunningAnimations, isTrue); + + await tester.pump(const Duration(milliseconds: 1100)); + expect(_opacity(tester), lessThan(1)); + + await tester.pump(_longerThanTheBeats); + expect(tester.hasRunningAnimations, isFalse); + expect(_opacity(tester), 1); + }); + + testWidgets('beats again when what it listens to notifies', (tester) async { + final activity = ValueNotifier(0); + addTearDown(activity.dispose); + await tester.pumpWidget(_app(Pulse(restartOn: activity, child: _dot))); + await tester.pump(_longerThanTheBeats); + expect(tester.hasRunningAnimations, isFalse); + + activity.value++; + await tester.pump(); + expect(tester.hasRunningAnimations, isTrue); + + await tester.pump(_longerThanTheBeats); + expect(tester.hasRunningAnimations, isFalse); + }); + + testWidgets('notifications during the beats do not stretch them', ( + tester, + ) async { + final activity = ValueNotifier(0); + addTearDown(activity.dispose); + await tester.pumpWidget(_app(Pulse(restartOn: activity, child: _dot))); + + // A call tells of itself several times as it goes. + for (var i = 0; i < 3; i++) { + await tester.pump(const Duration(milliseconds: 300)); + activity.value++; + } + await tester.pump(const Duration(seconds: 7)); + + expect(tester.hasRunningAnimations, isFalse); + }); + + testWidgets('does not beat while it is not active', (tester) async { + await tester.pumpWidget(_app(const Pulse(active: false, child: _dot))); + expect(tester.hasRunningAnimations, isFalse); + expect(_fade(), findsNothing); + + await tester.pumpWidget(_app(const Pulse(child: _dot))); + expect(tester.hasRunningAnimations, isTrue); + + await tester.pumpWidget(_app(const Pulse(active: false, child: _dot))); + expect(tester.hasRunningAnimations, isFalse); + }); + + testWidgets('stays still when the system asks for reduced motion', ( + tester, + ) async { + final activity = ValueNotifier(0); + addTearDown(activity.dispose); + await tester.pumpWidget( + _app(Pulse(restartOn: activity, child: _dot), reducedMotion: true), + ); + expect(tester.hasRunningAnimations, isFalse); + + activity.value++; + await tester.pump(); + expect(tester.hasRunningAnimations, isFalse); + }); +}