Skip to content
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

Add response cancellation function #1082

Open
glanium opened this issue Dec 17, 2023 · 2 comments
Open

Add response cancellation function #1082

glanium opened this issue Dec 17, 2023 · 2 comments
Assignees
Labels
package:cronet_http type-enhancement A request for a change that isn't a bug

Comments

@glanium
Copy link

glanium commented Dec 17, 2023

I mean Response cancellation than Request cancellation.
i.e. After received response, I wanna cancel response.

following is usecase

    final response = await c.send(request);
    if (response.contentLength != null && response.contentLength! > 100 * 1024 * 1024) { // <-- Big data then cancel
      final subscription = response.stream.listen((value) {});
      await subscription.cancel();
    }

In dart' stream, Stream can be cacelled by invoking subscription's cancel method.
Then StreamController will be notifed of cancel.

As long as i look at current crotnet_http implementaion, cancel request is ignored completely.

https://github.com/dart-lang/http/blob/c114aa06c510a28e942e9158711ed052b492c980/pkgs/cronet_http/lib/src/cronet_client.dart#L144C1-L152C26

  // The order of callbacks generated by Cronet is documented here:
  // https://developer.android.com/guide/topics/connectivity/cronet/lifecycle
  return jb.UrlRequestCallbackProxy_UrlRequestCallbackInterface.implement(
      jb.$UrlRequestCallbackProxy_UrlRequestCallbackInterfaceImpl(
    onResponseStarted: (urlRequest, responseInfo) {
      responseStream = StreamController(onCancel: () {  urlRequest.cancel();  }); // <---- Cancel?
      final responseHeaders =
          _cronetToClientHeaders(responseInfo.getAllHeaders());
      int? contentLength;

Add onCancel callback to StreamController then handle cancel request.

This applies to cupertino_http package if it support cancel.

HTTP2 supports multiplexing, and can force the stream to be closed by sending frame
RST_STREAM

In case of HTTP1, Cancel causes connection close?

thx.

@glanium glanium added package:cronet_http type-enhancement A request for a change that isn't a bug labels Dec 17, 2023
@glanium
Copy link
Author

glanium commented Dec 18, 2023

following might be safer implementation.

jb.UrlRequestCallbackProxy_UrlRequestCallbackInterface _urlRequestCallbacks(
    BaseRequest request, Completer<StreamedResponse> responseCompleter) {
  StreamController<List<int>>? responseStream;
  JByteBuffer? jByteBuffer;
  var numRedirects = 0;
  var cancelled;  // <-- flag

  // The order of callbacks generated by Cronet is documented here:
  // https://developer.android.com/guide/topics/connectivity/cronet/lifecycle
  return jb.UrlRequestCallbackProxy_UrlRequestCallbackInterface.implement(
      jb.$UrlRequestCallbackProxy_UrlRequestCallbackInterfaceImpl(
    onResponseStarted: (urlRequest, responseInfo) {
      responseStream = StreamController(onCancel: () => cancelled = true);  // <--- set flag
      final responseHeaders =
          _cronetToClientHeaders(responseInfo.getAllHeaders());
      int? contentLength;


      switch (responseHeaders['content-length']) {
        case final contentLengthHeader?
            when !_digitRegex.hasMatch(contentLengthHeader):
          responseCompleter.completeError(ClientException(
            'Invalid content-length header [$contentLengthHeader].',
            request.url,
          ));
          urlRequest.cancel();
          return;
        case final contentLengthHeader?:
          contentLength = int.parse(contentLengthHeader);
      }
      responseCompleter.complete(StreamedResponse(
        responseStream!.stream,
        responseInfo.getHttpStatusCode(),
        contentLength: contentLength,
        reasonPhrase: responseInfo
            .getHttpStatusText()
            .toDartString(releaseOriginal: true),
        request: request,
        isRedirect: false,
        headers: responseHeaders,
      ));


      jByteBuffer = JByteBuffer.allocateDirect(_bufferSize);
      urlRequest.read(jByteBuffer!);
    },
    onRedirectReceived: (urlRequest, responseInfo, newLocationUrl) {
      if (!request.followRedirects) {
        urlRequest.cancel();
        responseCompleter.complete(StreamedResponse(
            const Stream.empty(), // Cronet provides no body for redirects.
            responseInfo.getHttpStatusCode(),
            contentLength: 0,
            reasonPhrase: responseInfo
                .getHttpStatusText()
                .toDartString(releaseOriginal: true),
            request: request,
            isRedirect: true,
            headers: _cronetToClientHeaders(responseInfo.getAllHeaders())));
        return;
      }
      ++numRedirects;
      if (numRedirects <= request.maxRedirects) {
        urlRequest.followRedirect();
      } else {
        urlRequest.cancel();
        responseCompleter.completeError(
            ClientException('Redirect limit exceeded', request.url));
      }
    },
    onReadCompleted: (urlRequest, responseInfo, byteBuffer) {
     if (cancelled) {  
       urlRequest.cancel();                          // <-- cancel and cleanup here
       //responseStream!.sink.close();   
       byteBuffer.release();
     } else {
        byteBuffer.flip();
        responseStream!
            .add(jByteBuffer!.asUint8List().sublist(0, byteBuffer.remaining));
        byteBuffer.clear();
        urlRequest.read(byteBuffer);
      }
    },

@mohsinnaqvi606
Copy link

Any update on this?
This functionality is still missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:cronet_http type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants