Skip to content

add support for logging streamed data in http_logger #497

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

Open
saninthottungal wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add support for logging streamed data in http_logger #497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion packages/talker_http_logger/example/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ dependencies:
equatable: ^2.0.7

dev_dependencies:
flutter_lints: ^5.1.1
flutter_lints: ^6.0.0

dependency_overrides:
talker_http_logger:
Expand Down
47 changes: 40 additions & 7 deletions packages/talker_http_logger/lib/talker_http_logger_interceptor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:convert';

import 'package:http_interceptor/http_interceptor.dart';
import 'package:talker/talker.dart';
import 'package:talker_http_logger/http_error_log.dart';
Expand Down Expand Up @@ -103,32 +105,63 @@ class TalkerHttpLogger extends InterceptorContract {
}) async {
final String message = '${response.request?.url}';

switch (response.statusCode) {
BaseResponse resForTalker;
BaseResponse resForReturn;

if (response is StreamedResponse) {
final bytes = await response.stream.toBytes();
final body = utf8.decode(bytes);

@sourcery-ai sourcery-ai Bot May 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue: Consider handling non-UTF8 / binary responses more defensively when decoding.

utf8.decode(bytes) will throw on non‑UTF8 bodies (e.g. binary or malformed payloads), causing the interceptor to fail instead of just logging. Consider using utf8.decode(bytes, allowMalformed: true) or wrapping the decode in try/catch and falling back to a safer representation (e.g. hex or bytes.toString()) when decoding fails.

resForTalker = Response(
body,
response.statusCode,
headers: response.headers,
isRedirect: response.isRedirect,
persistentConnection: response.persistentConnection,
reasonPhrase: response.reasonPhrase,
request: response.request,
Comment on lines +112 to +121

@sourcery-ai sourcery-ai Bot May 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion (performance): Buffering the full streamed response body may be problematic for large payloads.

Reading the entire StreamedResponse into memory with toBytes() significantly increases memory usage and risks OOM for large or long-lived streams. Consider guarding this behavior (e.g., size limits, content-type checks, or a config flag to disable full-body logging for streamed responses) to avoid unbounded buffering.

Suggested implementation:

import 'dart:convert';

import 'package:talker/talker.dart';
import 'package:talker_http_logger/http_error_log.dart';

const int _kMaxLoggableStreamedResponseContentLength = 1024 * 1024; // 1 MiB


    if (response is StreamedResponse) {
      final contentLengthHeader = response.headers['content-length'];
      final contentLength = response.contentLength ??
          (contentLengthHeader != null ? int.tryParse(contentLengthHeader) : null);

      final shouldBufferBody =
          contentLength != null && contentLength <= _kMaxLoggableStreamedResponseContentLength;

      if (shouldBufferBody) {
        final bytes = await response.stream.toBytes();
        final body = utf8.decode(bytes);
        resForTalker = Response(
          body,
          response.statusCode,
          headers: response.headers,
          isRedirect: response.isRedirect,
          persistentConnection: response.persistentConnection,
          reasonPhrase: response.reasonPhrase,
          request: response.request,
        );

        resForReturn = StreamedResponse(
          Stream.value(bytes),
          response.statusCode,
          headers: response.headers,
          isRedirect: response.isRedirect,
          persistentConnection: response.persistentConnection,
          reasonPhrase: response.reasonPhrase,
          request: response.request,
        );
      } else {
        // Avoid buffering large or unknown-size streamed responses to prevent high memory usage.
        resForTalker = Response(
          '',
          response.statusCode,
          headers: response.headers,
          isRedirect: response.isRedirect,
          persistentConnection: response.persistentConnection,
          reasonPhrase: response.reasonPhrase,
          request: response.request,
        );

        // Do not consume the stream; pass the original response through.
        resForReturn = response;
      }

To make this behavior configurable (as hinted in the review), you may also want to:

  1. Add a configuration option to the interceptor to control _kMaxLoggableStreamedResponseContentLength (or to completely disable streamed-body logging).
  2. Use that configuration instead of the hard-coded 1 MiB constant so library consumers can tune or disable buffering based on their needs.

);

Comment on lines +114 to +123

@sourcery-ai sourcery-ai Bot May 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question (bug_risk): Re-wrapping StreamedResponse as Response for logging/filters changes the response type surface.

For streamed responses, resForTalker is now a Response instead of StreamedResponse, so any responseFilter/errorFilter relying on StreamedResponse (type checks or streaming-specific behavior) will no longer work as before. If this change isn’t intentional, consider preserving the original type for filters/logging (e.g., log metadata plus a separately captured body), or clearly document that streamed responses will appear as Response in the logger path.

resForReturn = StreamedResponse(
Stream.value(bytes),
response.statusCode,
headers: response.headers,
isRedirect: response.isRedirect,
persistentConnection: response.persistentConnection,
reasonPhrase: response.reasonPhrase,
request: response.request,
contentLength: bytes.length,
);
Comment on lines +130 to +133

@sourcery-ai sourcery-ai Bot May 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): Using bytes.length as contentLength may diverge from the original response semantics.

The original StreamedResponse may have an explicit contentLength (including -1 when unknown). Overwriting it with bytes.length can change behavior for consumers that rely on contentLength (e.g., progress, buffering). Prefer preserving response.contentLength when it’s non-null and non-negative, and only fall back to bytes.length when the original length is unknown.

Suggested change
reasonPhrase: response.reasonPhrase,
request: response.request,
contentLength: bytes.length,
);
reasonPhrase: response.reasonPhrase,
request: response.request,
contentLength: (response.contentLength != null &&
response.contentLength! >= 0)
? response.contentLength
: bytes.length,
);

} else {
resForTalker = response;
resForReturn = response;
}

switch (resForTalker.statusCode) {
case int statusCode when settings.enabled && statusCode < 400:
if (settings.responseFilter?.call(response) ?? true) {
if (settings.responseFilter?.call(resForTalker) ?? true) {
_talker.logCustom(
HttpResponseLog(
message,
response: response,
response: resForTalker,
settings: settings,
),
);
}
break;
case _ when settings.enabled:
if (settings.errorFilter?.call(response) ?? true) {
if (settings.errorFilter?.call(resForTalker) ?? true) {
_talker.logCustom(
HttpErrorLog(
message,
request: response.request,
response: response,
request: resForTalker.request,
response: resForTalker,
settings: settings,
),
);
}
break;
}

return response;
return resForReturn;
}
}
Loading