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

Empty response body is read as null #1834

Closed
jeiea opened this issue May 25, 2023 · 6 comments · Fixed by #1874
Closed

Empty response body is read as null #1834

jeiea opened this issue May 25, 2023 · 6 comments · Fixed by #1874
Assignees
Labels
b: regression Worked before but not now fixed p: dio Targeting `dio` package s: bug Something isn't working

Comments

@jeiea
Copy link

jeiea commented May 25, 2023

Package

dio

Version

5.1.2

Output of flutter doctor -v

[✓] Flutter (Channel stable, 3.10.2, on macOS 13.3.1 22E772610a darwin-arm64, locale en-KR)
    • Flutter version 3.10.2 on channel stable at /Users/jeiea/.asdf/installs/flutter/2.10.3-stable
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 9cd3d0d9ff (28 hours ago), 2023-05-23 20:57:28 -0700
    • Engine revision 90fa3ae28f
    • Dart version 3.0.2
    • DevTools version 2.23.1

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.2)
    • Android SDK at /Users/jeiea/Library/Android/sdk
    • Platform android-33, build-tools 33.0.2
    • Java binary at: /Users/jeiea/.asdf/shims/java
    • Java version OpenJDK Runtime Environment Temurin-11.0.15+10 (build 11.0.15+10)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14E222b
    • CocoaPods version 1.12.1

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[!] Android Studio
    • Android Studio at /Applications/Android Studio Preview.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    ✗ Unable to find bundled Java version.
    • Try updating or re-installing Android Studio.

[✓] VS Code (version 1.78.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.64.0

[✓] Connected device (3 available)
    • HojuniPad (mobile) • 00008101-001D44A63E29001E • ios            • iOS 16.4.1 20E252
    • macOS (desktop)    • macos                     • darwin-arm64   • macOS 13.3.1 22E772610a darwin-arm64
    • Chrome (web)       • chrome                    • web-javascript • Google Chrome 113.0.5672.126

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

! Doctor found issues in 1 category.

Dart Version

No response

Steps to Reproduce

Run the following dart source.

import 'package:dio/dio.dart';

Future<void> main() async {
  final dio = Dio();
  final response = await dio.get('https://jeiea.free.beeceptor.com/empty');
  final data = response.data;
  print(data);
}

Expected Result

Print empty string.

I found this from my app's breaking behavior so think v4 would return empty string in this situation.

Also I think regarding as empty string is correct because the response has text/html content type and 200 OK, not no content.

Actual Result

> dart run src/main.dart
null
@jeiea jeiea added h: need triage This issue needs to be categorized s: bug Something isn't working labels May 25, 2023
@kuhnroyal
Copy link
Member

@AlexV525 This was changed in cce167e - what do you think?

@AlexV525
Copy link
Member

@AlexV525 This was changed in cce167e - what do you think?

Yeah I saw this one. I was thinking to make it only modify the result to null only when the request and the response type is JSON.

@AlexV525 AlexV525 added p: dio Targeting `dio` package b: regression Worked before but not now and removed h: need triage This issue needs to be categorized labels Jun 15, 2023
@AlexV525 AlexV525 self-assigned this Jun 15, 2023
@AlexV525
Copy link
Member

@jeiea If the response type is defined as JSON (by default), in order to return a valid JSON response, the response body must be null, otherwise an empty string is an expected result for parsing a JSON. Setting the response type to others will keep the original result after #1874.

@jeiea
Copy link
Author

jeiea commented Jun 19, 2023

@AlexV525 I'm still receiving null with this pubspec and the original code snippet. I may have missed something, but could you double check?

dependencies:
  dio:
    git:
      url: https://github.com/cfug/dio.git
      path: dio
      ref: fix/null-response-body-only-when-response-type-json

@AlexV525
Copy link
Member

PR updated.

@bgenidy
Copy link

bgenidy commented Jun 25, 2023

made a comment on it, I think enabling users to easily keep back compatibility with Dio v4 decoding would significantly help reduce chances for apis to break post upgrade. Also this wasn't explicitly called out in the change logs for Dio v5, if possible it would be great to update documentation to include it as part of the breaking changes post v4 upgrade.

AlexV525 added a commit that referenced this issue Jul 10, 2023
…1874)

Resolves #1834.

### 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
@AlexV525 AlexV525 added the fixed label Jul 29, 2023
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 s: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants