Skip to content

Shelf 1.1.1 breaking change: request body runtime type changed from Stream<List<int>> to Stream<Uint8List> #189

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.

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

Closed
nex3 opened this issue May 18, 2021 · 7 comments
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@nex3
Copy link
Member

nex3 commented May 18, 2021

Shelf 1.1.1 introduced a breaking change. By changing the runtime type of request bodies from Stream<List<int>> to Stream<Uint8List>, it caused it to no longer work with instances of StreamTransformer<List<int>, List<int>> such as dart:io's gzip.encoder and gzip.decoder. This is a breaking change and, if it was necessary, should have been released with a major version bump.

The following code works with shelf 1.1.0 and not with 1.1.1:

import 'dart:io';

import 'package:shelf/shelf.dart' as shelf;
import 'package:shelf/shelf_io.dart' as shelf_io;

Future<void> main() async {
  var server = await shelf_io.serve((request) async {
      await request.read().transform(gzip.decoder).drain();
      return shelf.Response.ok("");
    }, "localhost", 0);

  var client = HttpClient();
  var request = await client.post("localhost", server.port, "/");
  await request.close();
  client.close(force: true);
  server.close();
}
@nex3 nex3 added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label May 18, 2021
@kevmoo
Copy link
Member

kevmoo commented May 18, 2021

CC @natebosch @jonahwilliams

@kevmoo
Copy link
Member

kevmoo commented May 18, 2021

Yeah, very unintentional.

Before we were ALWAYS wrapping the input in List<int> which turned out to be very expensive. Now we just flow through the input bits.

The question now: do we revert and give up the perf win?

This is a ~duplicate of #188 – leaving open to discuss

nex3 added a commit to google/dart_cli_pkg that referenced this issue May 18, 2021
@nex3
Copy link
Member Author

nex3 commented May 18, 2021

Whether or not you choose to release the cast-free implementation as 2.x, the 1.x branch should continue to cast() because doing otherwise is a breaking change.

nex3 added a commit to google/dart_cli_pkg that referenced this issue May 19, 2021
@kevmoo kevmoo changed the title Shelf 1.1.1 introduced a breaking change Shelf 1.1.1 breaking change: request body runtime type changed from Stream<List<int>> to Stream<Uint8List> Jun 18, 2021
@zambetpentru
Copy link

Hi, as raised in #193 I am trying to find the best way now to receive a large file upload. Is there a recommended example?

@kevmoo
Copy link
Member

kevmoo commented Jul 21, 2021

Just got to put the converter first:

diff --git a/example/mime.dart b/example/mime.dart
index 1975ead..a5735a9 100644
--- a/example/mime.dart
+++ b/example/mime.dart
@@ -6,7 +6,7 @@ import 'package:shelf/shelf_io.dart' as shelf_io;
 Future<void> main() async {
   var server = await shelf_io.serve(
     (request) async {
-      await request.read().transform(gzip.decoder).drain();
+      await gzip.decoder.bind(request.read()).drain();
       return shelf.Response.ok('');
     },
     'localhost',

@Silentdoer
Copy link

this bug has been fixed?

@kevmoo
Copy link
Member

kevmoo commented Mar 22, 2022

I doubt we're going to fix it at this point...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

4 participants