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

Make CancelToken be able to cancel response streams from ResponseType.stream requests #2025

Closed
tomekit opened this issue Nov 7, 2023 · 3 comments · Fixed by #2068
Closed
Labels
e: PR welcomed fixed p: dio Targeting `dio` package s: feature This issue indicates a feature request

Comments

@tomekit
Copy link

tomekit commented Nov 7, 2023

Package

dio

Version

5.3.3

Operating-System

Linux

Output of flutter doctor -v

[✓] Flutter (Channel stable, 3.13.9, on Ubuntu 23.04 6.2.0-36-generic, locale en_US.UTF-8)
    • Flutter version 3.13.9 on channel stable at /opt/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision d211f42860 (12 days ago), 2023-10-25 13:42:25 -0700
    • Engine revision 0545f8705d
    • Dart version 3.1.5
    • DevTools version 2.25.0

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.1)
    • Android SDK at /home/tomek/Android/Sdk
    • Platform android-34, build-tools 33.0.1
    • ANDROID_HOME = /home/tomek/Android/Sdk
    • Java binary at: /opt/android-studio/jbr/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)
    • All Android licenses accepted.

[✓] Chrome - develop for the web
    • Chrome at google-chrome

[✓] Linux toolchain - develop for Linux desktop
    • Ubuntu clang version 15.0.7
    • cmake version 3.25.1
    • ninja version 1.11.1
    • pkg-config version 1.8.1

[✓] Android Studio (version 2022.3)
    • Android Studio at /opt/android-studio
    • Flutter plugin version 75.1.2
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)

[✓] VS Code (version 1.78.2)
    • VS Code at /usr/share/code
    • Flutter extension can be installed from:
      🔨 https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter

[✓] Connected device (2 available)
    • Linux (desktop) • linux  • linux-x64      • Ubuntu 23.04 6.2.0-36-generic
    • Chrome (web)    • chrome • web-javascript • Google Chrome 114.0.5735.198

[✓] Network resources
    • All expected network resources are available.

• No issues found!

Dart Version

3.1.5

Steps to Reproduce

Run: https://github.com/tomekit/dio_cancel_issue_stream_get

or use below main.dart file:

main.dart
import 'package:dio/dio.dart';
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Dio cancellation issue',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurpleAccent),
        useMaterial3: true,
      ),
      home: const MyHomePage(title: 'Dio cancellation issue'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});
  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {

  int _downloaded = 0;
  bool _running = false;
  Exception? _failed;
  CancelToken cancelToken = CancelToken();

  @override
  Widget build(BuildContext context) {
    return Scaffold(
        appBar: AppBar(
          backgroundColor: Theme.of(context).colorScheme.inversePrimary,
          title: Text(widget.title),
        ),
        body: Center(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.center,
            children: <Widget>[
              Text('_downloaded: ${_downloaded}, _running: ${_running}, _failed: ${_failed?.toString()},')
            ],
          ),
        ),
        floatingActionButton:
        Stack(
          children: <Widget>[
            Align(
              alignment: Alignment.bottomLeft,
              child: FloatingActionButton(
                onPressed: () async {
                  fetch(ResponseType.bytes);
                },
                tooltip: 'Start bytes',
                child: const Icon(Icons.start),
              ),
            ),
            Align(
              alignment: Alignment.bottomCenter,
              child: FloatingActionButton(
                onPressed: () async {
                  fetch(ResponseType.stream);
                },
                tooltip: 'Start stream',
                child: const Icon(Icons.start),
              ),
            ),
            Align(
              alignment: Alignment.bottomRight,
              child: FloatingActionButton(
                onPressed: () => cancelToken.cancel(),
                tooltip: 'Cancel',
                child: const Icon(Icons.cancel),
              ),
            ),
          ],
        )
    );
  }

  fetch(ResponseType responseType) async {
    final url = "http://ash-speed.hetzner.com/100MB.bin";

    final dio = Dio();

    final options = Options(
      // responseType: ResponseType.bytes, // Works fine
      // responseType: ResponseType.stream, // No effect
      responseType: responseType
    );

    // For ResponseType.bytes purposes
    onReceiveProgress(int sent, int __) {
      setState(() {
        _downloaded = sent;
      });
      return;
    }

    try {

      setState(() {
        _running = true;
        _failed = null;
        cancelToken = CancelToken();
      });

      final response = await dio.get(url, options: options, cancelToken: cancelToken, onReceiveProgress: onReceiveProgress);
      final status = response.statusCode;

      var downloadedTotalStream = 0;
      if (status == 200) {
        await for (final List<int> dataChunk in response.data.stream) {
          downloadedTotalStream += dataChunk.length;
          setState(() {
            _downloaded = downloadedTotalStream;
          });
        }
      } else {
        setState(() {
          _failed = Exception("Status code: $status");
        });
      }

    } catch(e) {
      setState(() {
        _failed = e as Exception;
      });
    }

    setState(() {
      _running = false;
    });
  }
}

Click middle "Start stream" button to start streamed download, then click bottom right: "Cancel" button. Nothing happens in the release mode. In the debug mode your IDE (in my case Android Studio) may (it doesn't happen consistently) capture uncaught exception.
Screenshot from 2023-11-11 11-04-29

That's unexpected behavior, request doesn't get cancelled.

Click bottom left: "Start bytes" button to start bytes download, then click bottom right: "Cancel" button. DioException is thrown. That's expected behavior, request cancelling takes place.

Expected Result

DioException shall be thrown with type set to DioExceptionType.cancel even if: responseType: ResponseType.stream is used.

Actual Result

DioException isn't thrown when request with responseType: ResponseType.stream gets cancelled.

Test app:
image

@kuhnroyal
Copy link
Member

I agree, thanks for the reproduction. I think I have a solution but testing this is kinda hard.
Do you think #1953 is the same problem?

@kuhnroyal kuhnroyal added the p: dio Targeting `dio` package label Nov 12, 2023
@AlexV525
Copy link
Member

I've been investigated about the cause of the issue.

When we request stream responses, the response body returns immediately.

// Do not handled the body for streams.
if (responseType == ResponseType.stream) {
return responseBody;
}

When you get the response, the request flow is finished. Nothing in the queue was still awaiting, thus the cancel token is no longer effective (because the request is, by a specific aspect, ended). From the flow, we can say the behavior is as desired.

Given the above fact, because you have already obtained the stream, you can use other mechanisms rather than cancel tokens to stop the stream reading.

The issue is nothing related to #1953 and #2028. I understand everyone's issue is urgent to themselves, but referencing unrelated issues will cause redundant information for people who subscribed to those issues, and increase the complexibility for maintainers to find the root cause.

Still, I've tried to improve this and managed to throw the cancel exception for the stream.
image
But it doesn't look like a proper way to solve it because it depends on implementation in adapters. So I gonna leave this open and continue to investigate to see if we can have a proper solution.

@AlexV525 AlexV525 added s: feature This issue indicates a feature request e: PR welcomed and removed h: need triage This issue needs to be categorized s: bug Something isn't working labels Nov 13, 2023
@AlexV525 AlexV525 changed the title Cancel token doesn't work when: "ResponseType.stream" is used Make CancelToken be able to cancel response streams from ResponseType.stream requests Nov 13, 2023
@kuhnroyal kuhnroyal linked a pull request Nov 13, 2023 that will close this issue
7 tasks
@kuhnroyal
Copy link
Member

@AlexV525 I already had some changes and tests for this - see #2032

github-merge-queue bot pushed a commit that referenced this issue Dec 22, 2023
* fixes #2025

This is a new approach that handles both IO & HTTP/2 adapters.
I also moved the `receiveTimeout` handling for IO & HTTP2 to the new
helper.

### New Pull Request Checklist

- [x] I have read the
[Documentation](https://pub.dev/documentation/dio/latest/)
- [x] I have searched for a similar pull request in the
[project](https://github.com/cfug/dio/pulls) and found none
- [x] I have updated this branch with the latest `main` branch to avoid
conflicts (via merge from master or rebase)
- [x] I have added the required tests to prove the fix/feature I'm
adding
- [x] I have updated the documentation (if necessary)
- [x] I have run the tests without failures
- [x] I have updated the `CHANGELOG.md` in the corresponding package

### Additional context and info (if any)

<!-- Provide more context and info about the PR. -->

---------

Co-authored-by: Alex Li <github@alexv525.com>
@AlexV525 AlexV525 added the fixed label Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e: PR welcomed fixed p: dio Targeting `dio` package s: feature This issue indicates a feature request
Projects
None yet
3 participants