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

Connection closed before response was received #2170

Closed
Abouassi98 opened this issue Mar 28, 2024 · 23 comments · Fixed by #2173
Closed

Connection closed before response was received #2170

Abouassi98 opened this issue Mar 28, 2024 · 23 comments · Fixed by #2173
Labels
b: regression Worked before but not now fixed p: dio Targeting `dio` package platform: io s: bug Something isn't working valuable The issue or the fix means a lot to the library.

Comments

@Abouassi98
Copy link

Abouassi98 commented Mar 28, 2024

Package

dio

Version

5.4.2+1

Operating-System

Android, iOS

Adapter

Default Dio

Output of flutter doctor -v

[✓] Flutter (Channel stable, 3.19.4, on macOS 14.4.1 23E224 darwin-arm64, locale en-EG)
[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
[✓] Xcode - develop for iOS and macOS (Xcode 15.3)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2023.2)
[✓] VS Code (version 1.87.2)
[✓] Connected device (3 available)            
    ! Error: Browsing on the local area network for Me. Ensure the device is unlocked and attached with a cable or associated with the same
      local area network as this Mac.
      The device must be opted into Developer Mode to connect wirelessly. (code -27)
    ! Error: Browsing on the local area network for Mohamed’s Iphone (2). Ensure the device is unlocked and attached with a cable or
      associated with the same local area network as this Mac.
      The device must be opted into Developer Mode to connect wirelessly. (code -27)
    ! Error: Browsing on the local area network for ‏iPhone ‏Fadwa. Ensure the device is unlocked and attached with a cable or associated
      with the same local area network as this Mac.
      The device must be opted into Developer Mode to connect wirelessly. (code -27)
    ! Error: Browsing on the local area network for H. Sayed. Ensure the device is unlocked and attached with a cable or associated with the
      same local area network as this Mac.
      The device must be opted into Developer Mode to connect wirelessly. (code -27)
[✓] Network resources

• No issues found!

Dart Version

3.3.2

Steps to Reproduce

  1. Run with any version of Dio above 5.4.0.
  2. try to send more requests in a short period of time.

Expected Result

When using Dio version 5.4.0, the connection is not closed and the response succeeds.

untitled2

Actual Result

flutter: The following exception was thrown getBOKRechargeRequestStatusProvider:
flutter: AppException.serverException(type: ServerExceptionType.unknown, message: HttpException: Connection closed before response was received, uri = https://api.bravo-sudan.com/bravo/api/v1/bok/getRechargeRequests, code: null)
flutter:
#0 DioMixin.fetch (package:dio/src/dio_mixin.dart:509:7)

#1 MainApiFacade._errorHandler (package:bravo/core/infrastructure/network/main_api/api_callers/main_api_facade.dart:66:14)

#2 TransactionRemoteDataSource.getBOKRechargeRequestStatus (package:bravo/features/transaction/infrastructure/data_sourse/transaction_remote_remote_data_source.dart:48:20)

#3 TransactionHistoryRepository.getBOKRechargeRequestStatus (package:bravo/features/transaction/infrastructure/repository/transaction_history_repository.dart:43:20)

untitled

@Abouassi98 Abouassi98 added h: need triage This issue needs to be categorized s: bug Something isn't working labels Mar 28, 2024
@AlexV525
Copy link
Member

If this is version specific, could you run git bisect to find out which commit breaks the functionality?

@Abouassi98
Copy link
Author

Abouassi98 commented Mar 28, 2024

I have recently upgraded Dio from version 5.4.0 to 5.4.2+1. However, I believe the issue began with version 5.4.1.

@AlexV525
Copy link
Member

Running bisect will help to determine which exact commit breaks the behavior.

@Abouassi98
Copy link
Author

Abouassi98 commented Mar 28, 2024

I am contemplating running a bisect to pinpoint the problematic commit. However, I want to clarify that my recent commit solely consisted of upgrading the version to 5.4.2+1. Additionally, I have already tested version 5.4.1, as I suspect the issue originated there, and encountered the same problem.

@AlexV525
Copy link
Member

I understand your statement clearly, IMO. I'm asking you to do the bisect because you have a reproducible example and can debug the issue easily. Besides, simply switching between versions might not be helpful since every version includes a bunch of commits.

@Abouassi98
Copy link
Author

I've completed running git bisect to identify the commit that introduced the issue in the Dio package. After testing multiple commits, I've found that the first bad bisect commit is "Fix streamed response cancellation and progress (#2068)".

845e06f

@AlexV525
Copy link
Member

I've completed running git bisect to identify the commit that introduced the issue in the Dio package. After testing multiple commits, I've found that the first bad bisect commit is "Fix streamed response cancellation and progress (#2068)".

845e06f

Thanks for the bisect! Now could you split a minimal reproducible example so we can take a deeper investigation with your case?

@AlexV525 AlexV525 added h: need more info Further information is requested p: dio Targeting `dio` package b: regression Worked before but not now platform: io and removed h: need triage This issue needs to be categorized labels Mar 29, 2024
@AlexV525
Copy link
Member

cc @kuhnroyal to check the implementation as the code owner

@Abouassi98
Copy link
Author

Sure! Here's the minimal reproducible example: Minimal Reproducible Example.

@Abouassi98
Copy link
Author

@AlexV525, do you have any updates there?

@AlexV525
Copy link
Member

@AlexV525, do you have any updates there?

We'll let you know if any updates. We generally works when we have spare time since this is an open-source project.

@kuhnroyal
Copy link
Member

Sure! Here's the minimal reproducible example: Minimal Reproducible Example.

Thanks, sadly this is not minimal and requires to start and debug a flutter app. Can you reduce the actual request to a unit test or at least a simple Dart file?

@Abouassi98
Copy link
Author

Abouassi98 commented Apr 3, 2024

I'm certain this issue stems from using Riverpod, a common example in the docs.

final apiProvider = FutureProvider.autoDispose.family<List<dynamic>, String>((ref, endPoint) async {
  final cancelToken = CancelToken();
  ref.onDispose(() => cancelToken.cancel());
  final response = await ref.watch(dioProvider).get(endPoint, cancelToken: cancelToken);
  return response.data;
});

This issue happens with any version of Dio > 5.4.0, as I mentioned above.

And this is a Minimal Reproducible Example:

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

void main() {
  runApp(const ProviderScope(child: App()));
}

class App extends StatelessWidget {
  const App({super.key});

  @override
  Widget build(BuildContext context) => const MaterialApp(
        home: TabBarScreen(),
      );
}

class TabBarScreen extends StatefulWidget {
  const TabBarScreen({super.key});

  @override
  State<TabBarScreen> createState() => _TabBarScreenState();
}

class _TabBarScreenState extends State<TabBarScreen> with SingleTickerProviderStateMixin {
  late final TabController controller;
  @override
  void initState() {
    controller = TabController(
      length: 2,
      vsync: this,
    );

    super.initState();
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(),
      body: Column(
        mainAxisSize: MainAxisSize.min,
        crossAxisAlignment: CrossAxisAlignment.start,
        children: [
          TabBar(
            indicator: const BoxDecoration(),
            padding: const EdgeInsets.all(8),
            controller: controller,
            tabs: const [
              Tab(
                text: 'Posts',
              ),
              Tab(
                text: 'Users',
              ),
            ],
          ),
          Expanded(
            child: TabBarView(
              controller: controller,
              children: const [
                TabBarViewItem(
                  key: PageStorageKey<String>('tap1'),
                  endPoint: 'posts',
                ),
                TabBarViewItem(
                  key: PageStorageKey<String>('tap2'),
                  endPoint: 'users',
                ),
              ],
            ),
          ),
        ],
      ),
    );
  }
}

final dioProvider = StateProvider<Dio>((ref) {
  return Dio()
    ..options = BaseOptions(
      baseUrl: 'https://gorest.co.in/public/v2/',
      connectTimeout: const Duration(seconds: 20),
      receiveTimeout: const Duration(seconds: 30),
    );
});

final apiProvider = FutureProvider.autoDispose.family<List<dynamic>, String>((ref, endPoint) async {
  final cancelToken = CancelToken();
  ref.onDispose(() => cancelToken.cancel());
  final response = await ref.watch(dioProvider).get(endPoint, cancelToken: cancelToken);
  return response.data;
});

class TabBarViewItem extends ConsumerWidget {
  const TabBarViewItem({
    super.key,
    required this.endPoint,
  });
  final String endPoint;
  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final list = ref.watch(
      apiProvider(endPoint),
    );
    return Column(
      children: [
        list.when(
          data: (e) => Expanded(
            child: ListView.separated(
              padding: EdgeInsets.zero,
              itemBuilder: (context, index) {
                return Padding(
                  padding: const EdgeInsets.all(8.0),
                  child: Text(
                    e[index]['id'].toString(),
                  ),
                );
              },
              separatorBuilder: (context, index) {
                return const Divider();
              },
              itemCount: e.length,
            ),
          ),
          error: (obj, st) {
            return Container(
              height: 160,
              alignment: Alignment.center,
              child: ElevatedButton(
                onPressed: () => ref.invalidate(apiProvider),
                child: const Text(
                  'Retry',
                  style: TextStyle(color: Colors.white),
                  textAlign: TextAlign.center,
                ),
              ),
            );
          },
          loading: () {
            return Expanded(
              child: ListView.separated(
                itemBuilder: (context, index) =>
                    const Center(child: CircularProgressIndicator.adaptive()),
                separatorBuilder: (context, index) => const SizedBox(height: 24),
                itemCount: 4,
              ),
            );
          },
        ),
      ],
    );
  }
}

@AlexV525
Copy link
Member

AlexV525 commented Apr 3, 2024

This is actually not caused by #2068, but a very old implementation that detaches the socket that has been promoted recently in #2036.

TL;DR, the cause is:

responseStream.detachSocket().then((socket) => socket.destroy());

Details

ResponseBody.onClose was implemented to detach from the socket which provided its connection in the IOHttpClientAdapter. However, with the same host between requests, the connections are implied to use the same socket (which is reasonable). When the CancelToken got canceled, it would call ResponseBody.onClose and the socket detached, all requests related to the same host would be closed.

The solution here might be to remove the detaching which has no affection according to its doc: https://api.dart.dev/stable/3.3.3/dart-io/HttpClientResponse/detachSocket.html

This is normally used when a HTTP upgrade is negotiated and the communication should continue with a different protocol.

Anyway, this could be a long-affected regression, and the stack trace did not help us to combine them together, the case is rare and valuable!

@AlexV525 AlexV525 added valuable The issue or the fix means a lot to the library. and removed h: need more info Further information is requested labels Apr 3, 2024
@EArminjon
Copy link

What is the recommandation to avoid this issue ?

@kuhnroyal
Copy link
Member

This was changed in 5.4.1, you might be able to lock the dio version to 5.4.0.

@devguusta

This comment was marked as duplicate.

@Wratheus
Copy link

Wratheus commented Apr 10, 2024

The issue occurs when you have two requests with different CancelTokens, and when the first request is completed, the second request starts executing. At that moment, if you cancel the CancelToken of the first request somewhere in the code (for example, in Bloc dispose), then the second request gets canceled before it is received, even if it had a different cancelToken from the first request, causing the entire DIO connection to close.

For example

List<CancelToken> cancelTokens = [CancelToken(), CancelToken()]
RequestData request1 = await Repo.request1(cancelTokens.first); // done
RequestData request2 = Repo.request2(cancelTokens.last);  // no await
microtask({
    cancelTokens.first.cancel();  // parallel cancel first cancelToken
});

While request2 is still being execute, a cancelation occurs on the first CancelToken in parallel in code, which will drop request2. that cancelation should not influence second request.

Fixes I found so far:

  1. You can use lower Dio version (I rollbacked to 5.3.3, did not checked later versions yet).
  2. Open new Dio connection for each request.
  3. Initialize DIO with (BaseOptions(persistenConnection: false)). I think this option is the same meaning as new dio client for new request.

@kuhnroyal
Copy link
Member

@Wratheus Can you reproduce this in a test case? Also not sure if this is the same issue.

This seems to work:

    test('cancel one request', () {
      final dio = Dio()..options.baseUrl = 'https://httpbun.com';
      final token = CancelToken();

      expectLater(
        dio.get(
          '/drip?delay=0&duration=2&numbytes=10',
          cancelToken: token,
        ),
        throwsDioException(DioExceptionType.cancel),
      );
      expectLater(
        dio.get(
          '/drip?delay=0&duration=2&numbytes=10',
          cancelToken: CancelToken(),
        ),
        completes,
      );

      Future.delayed(const Duration(seconds: 1), () {
        token.cancel('cancelled');
      });
    });

@AlexV525
Copy link
Member

This case fails.

test(
'not closing sockets with requests that have same hosts',
() async {
final token = CancelToken();
final completer = Completer<Response?>();
// Complete the first request with the cancel token.
await dio.get('/get', cancelToken: token);
// Request the second without any cancel token, but with the same host.
dio.get('/drip?duration=3').then(
(res) {
completer.complete(res);
return res;
},
onError: (e) {
completer.complete(null);
return Response(requestOptions: (e as DioException).requestOptions);
},
);
// Simulate connection established.
await Future.delayed(const Duration(seconds: 1));
token.cancel();
final response = await completer.future;
// Response should be obtained without exceptions.
expect(response, isNotNull);
},
testOn: '!browser',
);

@Wratheus
Copy link

Wratheus commented Apr 11, 2024

There is no difference if there is cancelToken in second request or no. Same with post requests. That's the problem I was talking about. Issue does not occurs at 5.3.3 version

import 'dart:async';

import 'package:dio/dio.dart';
import 'package:flutter_test/flutter_test.dart';

  void main() {
    Dio dio = Dio();
    test(
      'not closing sockets with requests that have same hosts',
          () async {
        final token = CancelToken();
        const String baseUrl = 'https://httpbun.com';

        final completer = Completer<Response?>();
        // Complete the first request with the cancel token.
        await dio.get('$baseUrl/get', cancelToken: token);
        // Request the second without any cancel token, but with the same host.
        dio.get('$baseUrl/drip?duration=3', cancelToken: CancelToken()).then(
              (res) {
            completer.complete(res);
            return res;
          },
          onError: (e, t) {
                print(e);
                print(t);
            completer.complete(null);
            return Response(requestOptions: (e as DioException).requestOptions);
          },
        );
        // Simulate connection established.
        await Future.delayed(const Duration(seconds: 1));
        token.cancel();
        final response = await completer.future;
        // Response should be obtained without exceptions.
        expect(response, isNotNull);
      },
      testOn: '!browser',
    );
  }

@chesongsong
Copy link

5.4.0

Is there a big difference between 5.4.0 and 5.4.2+1? I also reproduced this problem on 5.4.2+1. If I lock 5.4.0, does my user need to make API level adjustments?

@AlexV525
Copy link
Member

The fix is included in v5.4.3 and shall not require interface updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b: regression Worked before but not now fixed p: dio Targeting `dio` package platform: io s: bug Something isn't working valuable The issue or the fix means a lot to the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants