Dio Web: Migration from XMLHttpRequest to new fetch api #2376
Conversation
|
Part of this code was likely copied from here: https://github.com/Zekfad/fetch_api/blob/dee32249d9ecb4bf6b4eb05062930b5d704423c9/lib/src/js_promise_or.dart |
|
Indeed, what would you suggest? Either rewriting Additionally, the bindings from final streamReader = ReadableStream(
ReadableStreamSource.fromStream(
requestStream.map((e) {
[...]
sentBytes += e.lengthInBytes;
if (options.onSendProgress != null) {
options.onSendProgress!(sentBytes, -1);
}
return e.toJS;
}),
),
);The dart doc https://pub.dev/documentation/web/latest/web/ReadableStream-extension-type.html shows the following method signature: The mdn webdocs show this example: https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream#examples: return new ReadableStream({
start(controller) {
[...]
},
});Sadly I was not able to implement this this way. That's way I chose those auto generated files. |
|
Just add short notice in header in borrowed files, that's sufficient enough. I thought by now package:web would have more coverage, since it's generated from webdl as opposed to fetch_api which was made by hand. Also good job for bringing fetch implementation for Dio, hope they'll accept your PR! |
|
Yes, also hope for an accept of my pr because I actually need this feature myself 😄 |
|
I think I need some help. I can't get my head around with the tests, they are so confusing. As an example:
test('POST with onSendProgress is preflighted', () async {
// If there is a preflight (OPTIONS) request, the server fails it
// by responding with status 418. This fails CORS, so the browser
// never sends the main request and this code throws.
expect(
() async {
final _ = await dio.post(
'/status/418',
data: 'body text',
options: Options(
validateStatus: (status) => true,
contentType: Headers.textPlainContentType,
),
onSendProgress: (_, __) {},
);
},
throwsDioException(
DioExceptionType.connectionError,
stackTraceContains: 'test/test_suite_test.dart',
),
);
});Which gets thrown in the code: // Check CORS
if (response.status == 418) {
if (options.onSendProgress != null || sendTimeout > Duration.zero || (request.method != 'GET' && options.contentType != Headers.textPlainContentType)) {
completer.completeError(
DioException.connectionError(
requestOptions: options,
error: 'CORS preflight request failed',
reason: 'The preflight request responded with status 418',
),
StackTrace.current,
);
}
}Which the test logs confirm: [dio]: 00:45 +93 -4: test/test_suite_test.dart: CORS preflight POST with onSendProgress is preflighted [E]
[dio]: DioException [connection error]: The connection errored: The preflight request responded with status 418 This indicates an error which most likely cannot be solved by the library.
[dio]: Error: CORS preflight request failed
[dio]: org-dartlang-sdk:///lib/_internal/js_runtime/lib/core_patch.dart 788:58 StackTrace.current
[dio]: package:dio_web_adapter/src/adapter.dart 238:22 BrowserHttpClientAdapter.<fn>
[dio]: org-dartlang-sdk:///lib/_internal/js_runtime/lib/async_patch.dart 311:19 _wrapJsFunctionForAsync.<fn>
[dio]: org-dartlang-sdk:///lib/_internal/js_runtime/lib/async_patch.dart 336:23 call
[dio]: package:stack_trace _run
[dio]: org-dartlang-sdk:///lib/_internal/js_runtime/lib/async_patch.dart 287:19 call
[dio]: org-dartlang-sdk:///lib/async/zone.dart 1422:46 _rootRunUnary
[dio]: org-dartlang-sdk:///lib/async/zone.dart 1323:34 runUnary
[dio]: org-dartlang-sdk:///lib/async/future_impl.dart 169:29 call
[dio]: org-dartlang-sdk:///lib/async/future_impl.dart 931:13 _Future._propagateToListeners
[dio]: org-dartlang-sdk:///lib/async/future_impl.dart 707:5 _completeWithValue
[dio]: org-dartlang-sdk:///lib/async/future_impl.dart 777:7 call
[dio]: package:stack_trace call
[dio]: org-dartlang-sdk:///lib/async/zone.dart 1414:12 _rootRun
[dio]: org-dartlang-sdk:///lib/async/zone.dart 1316:34 run
[dio]: org-dartlang-sdk:///lib/async/zone.dart 1225:7 runGuarded
[dio]: org-dartlang-sdk:///lib/async/zone.dart 1265:18 call
[dio]: org-dartlang-sdk:///lib/async/schedule_microtask.dart 40:12 _microtaskLoop
[dio]: org-dartlang-sdk:///lib/async/schedule_microtask.dart 49:5 _startMicrotaskLoop
[dio]: org-dartlang-sdk:///lib/_internal/js_runtime/lib/async_patch.dart 56:10 call
[dio]: ===== asynchronous gap ===========================
But the test still fails 😕 |
|
Checking the workflow seems some other exceptions were thrown: |
|
Indeed thanks for the hint. Although I still can't figure out how to make the CORS Tests succeed |
|
Could you also update the branch so the workflow can run? |
Signed-off-by: Binozo <70137898+Binozo@users.noreply.github.com>
|
sure |
There was a problem hiding this comment.
Could you update the adapter_impl.dart?
There was a problem hiding this comment.
Yes somehow I messed up the merge really bad 🙂
Is it now right?
|
@Zekfad, what is your idea about these borrowed files? It seems the PR borrows a lot. Would creating a new adapter to bridge the fetch_api and dio be sufficient? |
|
I'm keeping On a side note, I'm currently implementing a feature check for request streaming (it can't check if server is HTTP/3, but it can at least ensure that browser have the capability) and there is that discrepancy of closing dart stream after If you're strongly against adding a new dependency, since its ISC licensed I'd ok with just copying relevant files as long as attribution is retained. |
@Zekfad: Thanks for keeping the library updated! What I was trying to say is we should probably make a plugin like |
|
I had the same idea about adding |
@Binozo: Would you be able to create a separate PR that bridges the |
|
Hi, was there any decision made yet? @Binozo Any disussion or something is open to follow the solutions? I am using Dio for my projects. I am really looking forward to use fetch_client with Dio for streaming SSE. |
|
Hi @adar2378 I didn't have time to make a decision yet, I proceeded to use this way instead: import 'package:http/http.dart' as http;
final dio = Dio();
final client = http.Client();
final dioAdapter = ConversionLayerAdapter(client);
dio.httpClientAdapter = dioAdapter; |
|
@Binozo I would be very grateful if you could point me to the ConversationLayerAdapter ? I imagine it would require to write an adapter following this: |
|
@Binozo |
|
@adar2378 no this package is not meant to be used along with the Also if you want to create a |
Hello 👋
Since I wanted to make use of SSE but since #1740 was in the way I migrated the dio web adapter from XMLHttpRequest to the new fetch api
New Pull Request Checklist
mainbranch to avoid conflicts (via merge from master or rebase)CHANGELOG.mdin the correspondin 8000 g package