Adding dart:io client #82
Conversation
lib/src/io_client.dart
Outdated
| return request.url; | ||
| } | ||
|
|
||
| var location = redirects.last.location; |
There was a problem hiding this comment.
Actually I might just have to run through the array in case there was some ping ponging between servers. Not likely but certainly possible.
There was a problem hiding this comment.
Why wouldn't the last location be correct?
There was a problem hiding this comment.
Hypothetically if you called to a.com/foo that could redirect to another server b.com/bar which then does a relative redirect to b.com/baz.
I think I just need to run through the array with the lastLocation and use that to get the final url.
There was a problem hiding this comment.
Can you do p.url.joinAll() for that?
There was a problem hiding this comment.
@nex3 did not know about p.url.joinAll() is that still valid with the updated implementation?
There was a problem hiding this comment.
The new implementation looks fine as-is.
lib/src/io_client.dart
Outdated
| Future<StreamedResponse> send(BaseRequest request) async { | ||
| var stream = request.finalize(); | ||
| Future<Response> send(Request request) async { | ||
| var stream = await request.read(); |
There was a problem hiding this comment.
await doesn't do anything here since this is a Stream, right?
lib/src/io_client.dart
Outdated
| request.headers.forEach((name, value) { | ||
| ioRequest.headers.set(name, value); | ||
| }); | ||
|
|
||
| var response = await stream.pipe( | ||
| DelegatingStreamConsumer.typed(ioRequest)); | ||
| DelegatingStreamConsumer.typed<List<int>>(ioRequest) | ||
| ) as HttpClientResponse; |
There was a problem hiding this comment.
Rather than casting here, just await ioRequest.done.
lib/src/io_client.dart
Outdated
| isRedirect: response.isRedirect, | ||
| persistentConnection: response.persistentConnection, | ||
| reasonPhrase: response.reasonPhrase); | ||
| context: context); |
There was a problem hiding this comment.
I don't think it's correct to send the request's context back down the chain as part of the response.
There was a problem hiding this comment.
removed so assuming that it should just be an empty context
lib/src/io_client.dart
Outdated
| @@ -72,3 +73,16 @@ class IOClient extends BaseClient { | |||
| _inner = null; | |||
| } | |||
| } | |||
|
|
|||
| Uri _responseUrl(Request request, HttpClientResponse response) { | |||
There was a problem hiding this comment.
This should be part of the class, and also documented.
There was a problem hiding this comment.
K you want it static right? Was just following along with what we we're doing with free functions in message.dart
There was a problem hiding this comment.
K you want it static right?
No, as an instance method. I don't like making things static unless they have to be.
Was just following along with what we we're doing with free functions in
message.dart
Those are used in constructors, which is pretty different. Still, I might make them static methods.
lib/src/io_client.dart
Outdated
| return request.url; | ||
| } | ||
|
|
||
| var location = redirects.last.location; |
There was a problem hiding this comment.
Why wouldn't the last location be correct?
lib/src/io_client.dart
Outdated
| var location = redirects.last.location; | ||
|
|
||
| // Redirect can be relative or absolute | ||
| return (location.isAbsolute) ? location : request.url.resolveUri(location); |
|
@nex3 think everything is resolved now
|
?
|
🎊 thanks @nex3 ! Onto browser_client now |
Implements a
dart:ioclient with the new interface.