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

Bad state: Can't finalize a finalized MultipartFile #15

Open
aweiand opened this issue Oct 17, 2022 · 12 comments
Open

Bad state: Can't finalize a finalized MultipartFile #15

aweiand opened this issue Oct 17, 2022 · 12 comments

Comments

@aweiand
Copy link

aweiand commented Oct 17, 2022

Hi,

I'm using the latest packages:

dio: ^4.0.6
dio_smart_retry: ^1.3.2

When I post a file to the server and, it accepts only after first attemp, the error occours... here a simplified code:

    Dio dio = Dio(BaseOptions(
      baseUrl: ("https://$domain$base"),
    );

    dio.interceptors.add(
        RetryInterceptor(
          dio: dio,
          logPrint: print, // specify log function (optional)
          retries: 3, // retry count (optional)
          retryDelays: const [
            // set delays between retries (optional)
            Duration(seconds: 1), // wait 1 sec before first retry
            Duration(seconds: 2), // wait 2 sec before second retry
            Duration(seconds: 3), // wait 3 sec before third retry
          ],
        ),
      );

    final bytes = await cameraFile!.readAsBytes();
    final MultipartFile file = MultipartFile.fromBytes(bytes, filename: "teste");
    MapEntry<String, MultipartFile> a = MapEntry('teste', file);

    var options = FormData.fromMap({
      "bla": "bla",
    });

    options.files.add(a);

    var data = await dio.post("/post_test", data: options);
@vimaxwell
Copy link
Contributor

please test my PR #16

@aweiand
Copy link
Author

aweiand commented Nov 22, 2022

Hi @vimaxwell ,

After test in a test server I own, the issue persists...

@wolru
Copy link
Contributor

wolru commented Nov 26, 2022

@aweiand please test this PR
#17

I'm not using this library, so I edited with web editor.
If there is problem, let me know

@aweiand
Copy link
Author

aweiand commented Nov 29, 2022

@wolru , unfortunately the PR #17 don't fix the issue....

Although, the PR occasionally the error occurs on the first, second... attempt, not only on the last

Here is the stacktrace

#0      MultipartFile.finalize (package:dio/src/multipart_file.dart:133:7)
#1      FormData.finalize.<anonymous closure> (package:dio/src/form_data.dart:164:43)
#2      Future.forEach.<anonymous closure> (dart:async/future.dart:646:26)
#3      Future.doWhile.<anonymous closure> (dart:async/future.dart:703:26)
#4      _RootZone.runUnaryGuarded (dart:async/zone.dart:1586:10)
#5      _RootZone.bindUnaryCallbackGuarded.<anonymous closure> (dart:async/zone.dart:1625:26)
#6      Future.doWhile (dart:async/future.dart:719:18)
#7      Future.forEach (dart:async/future.dart:644:12)
#8      FormData.finalize (package:dio/src/form_data.dart:161:12)
#9      DioMixin._transformData (package:dio/src/dio_mixin.dart:730:23)
#10     DioMixin._dispatchRequest (package:dio/src/dio_mixin.dart:656:26)
#11     DioMixin.fetch.<anonymous closure> (package:dio/src/dio_mixin.dart:605:7)
#12     DioMixin.fetch._requestInterceptorWrapper.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:dio/src/dio_mixin.dart:517:28)
#13     DioMixin.checkIfNeedEnqueue (package:dio/src/dio_mixin.dart:789:22)
#14     DioMixin.fetch._requestInterceptorWrapper.<anonymous closure>.<anonymous closure> (package:dio/src/dio_mixin.dart:515:22)
#15     new Future.<anonymous closure> (dart:async/future.dart:253:37)
#16     Timer._createTimer.<anonymous closure> (dart:async-patch/timer_patch.dart:18:15)
#17     _Timer._runTimers (dart:isolate-patch/timer_impl.dart:398:19)
#18     _Timer._handleMessage (dart:isolate-patch/timer_impl.dart:429:5)
#19     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:192:12)

And here, is the screenshot of the error with the original plugin's code:
image

@aweiand
Copy link
Author

aweiand commented Nov 29, 2022

Ow, this is the PHP code (in Laravel) that I'm using to test (inside a controller and api route):

public function post_test(Request $request)
    {
        try {
            $file = $request->file('teste');
            $test = session('post_test');

            if($test == null || session('post_test') == 1){
                session('post_test') == null ? $request->session()->put('post_test', 1) : 
                                                $request->session()->put('post_test', 2);

                Log::alert('LOG 525', [
                    'files' => $file,
                    'request' => $request->all()
                ]);

                return response()->json([
                    'msg' => 'error',
                    'sess' => session('post_test')
                ], 525);
            } else {
                $request->session()->put('post_test', null);

                Log::alert('LOG 200', [
                    'files' => $file,
                    'request' => $request->all()
                ]);

                return response()->json([
                    'msg' => 'uhuu',
                    'sess' => session('post_test')
                ], 200);
            }
        } catch (\Exception $e) {
            Log::critical($e->getMessage(), ['file' => $e->getFile(), 'line' => $e->getLine(), 'request' => $request->all()]);
        }
    }

@rodion-m rodion-m reopened this Dec 1, 2022
@rodion-m
Copy link
Owner

rodion-m commented Dec 1, 2022

Hey @aweiand ! I've added the fix for this issue. Now to enable retrying you should add multipart data like this (using MultipartFileRecreatable):

    final formData =
        FormData.fromMap({'file': MultipartFileRecreatable.fromFileSync('README.md')});
      await dio.post<dynamic>(
        'https://multipart.free.beeceptor.com/post500',
        data: formData,
      );

See the full example in the test:

test('Retry for MultipartFileRecreatable is retrying', () async {
final dio = Dio();
const retries = 2;
final evaluator = DefaultRetryEvaluator(defaultRetryableStatuses);
dio.interceptors.add(
RetryInterceptor(
dio: dio,
logPrint: print,
retries: retries,
retryDelays: const [Duration(seconds: 1), Duration(seconds: 1)],
retryEvaluator: evaluator.evaluate,
),
);
final formData = FormData.fromMap({
'file': MultipartFileRecreatable.fromFileSync('README.md'),
});
try {
await dio.post<dynamic>(
'https://multipart.free.beeceptor.com/post500',
data: formData,
);
} on DioError catch (error) {
if (error.type != DioErrorType.response) {
rethrow;
}
}
expect(evaluator.currentAttempt, retries);
});

@aweiand Could you please check your case again using src from the current repo?

@rodion-m
Copy link
Owner

rodion-m commented Dec 1, 2022

It's the best solution I've found for today, but it's not very good, because it's not supported MultipartFile class, since this class doesn't have a filepath field or even a public stream. I hope dio developers will add any solution for this issue that will allow rewinding a multipart data stream. Related issue: cfug/dio#482

@rodion-m
Copy link
Owner

rodion-m commented Dec 2, 2022

While dio doesn't support MultipartFile recreation I want to consult with the community - should RetryInterceptor throw an exception before sending a request? I think this is the right behavior, but it'll break backward compatibility. At the moment I'll add just a warning to the log about the issue. If somebody has thoughts about it - feel free to share it in this thread.

@aweiand
Copy link
Author

aweiand commented Dec 2, 2022

@rodion-m, in a short test, it was suceffull! I will design a full test in the sequence. Thank's!!

In my opinion you don't has to throw an exception generically, maybe only on some listed error code?

@rodion-m
Copy link
Owner

rodion-m commented Dec 2, 2022

in a short test, it was suceffull! I will design a full test in the sequence. Thank's!!

Ok, thank you for testing!

In my opinion you don't has to throw an exception generically, maybe only on some listed error code?

If formdata has a type MultipartFile then the exception will be thrown anyway on retry (because a retry is not possible for MultipartFile).

@helloalbin
Copy link

Is It possible to add support for fromString method as well? Currently if we have both data as well as Files in the same request, retry is not working. My current code is

MultipartFile jsonData = MultipartFile.fromString(
          jsonEncode(data),
          contentType: http_parser.MediaType.parse('application/json'),
        );

So can we have an option like

MultipartFile jsonData = MultipartFileRecreatable.fromString(
          jsonEncode(data),
          contentType: http_parser.MediaType.parse('application/json'),
        );

@rodion-m
Copy link
Owner

Hey @helloalbin ! Certainly, feel free to add it as a PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants