tink_http icon indicating copy to clipboard operation
tink_http copied to clipboard

Client upload progress

Open kevinresol opened this issue 8 years ago • 9 comments

Suggest changing the api to:

function request(request:OutgoingRequest): Signal<Progress<IncomingResponse>>

and enum Progress<T> { Done(data:<T>); Progress(fraction:Float); Failed(error:Error);}

kevinresol avatar Dec 07 '16 10:12 kevinresol

or to make sure the response won't get lost, request could return:

{
  progress: Signal<Float>,
  repsonse: Future<IncomingResponse>,
}

kevinresol avatar Dec 07 '16 10:12 kevinresol

Hm... should even split to uploadProgress and downloadProgress

kevinresol avatar Dec 07 '16 10:12 kevinresol

and need a way to abort

kevinresol avatar Dec 07 '16 10:12 kevinresol

Progress is tricky, because in either direction content-length needn't be set. I guess this is roughly the best we can do:

interface HttpTransaction {
  var bytesWritten(get, never):Observable<Int>;
  var bytesRead(get, never):Observable<Int>;
  var response(get, never):Future<IncomingResponse>;
  function abort():Future<Bool>;
}

Aborting is going to be a PITA, I can tell you. A lot of fun stuff like ensuring the body of the OutgoingRequest is properly closed to avoid keeping files open and what not. But sure, it can be done :)

back2dos avatar Dec 07 '16 11:12 back2dos

Can we still include content length and make it Optional?

kevinresol avatar Dec 07 '16 11:12 kevinresol

Now we have tink.state.Progress, any new API proposal here?

kevinresol avatar Mar 06 '19 15:03 kevinresol

I think it is just:

function request(req:OutgoingRequest):Progress<Outcome<IncomingResponse, Error>>;

kevinresol avatar Nov 26 '20 06:11 kevinresol

Hmm. Looking at the signature, I would probably expect to the progress to report on the download, rather than the upload.

I'm still inclined to think that we might want to return a more complex object that has an upload and download progress and the response. Or it could be something like:

function request(req:OutgoingRequest, ?handlers:{ ?upload:ProgressValue->Void, ?download:ProgressValue->Void }):Progress<Outcome<IncomingResponse, Error>>;

Still doesn't really handle the topic of cancelation though.

back2dos avatar Nov 26 '20 17:11 back2dos

I wrongly thought that the response is only available after the upload is finished but it should not be the case. I think HTTP can produce a response header as soon as the request header is received, without the completely receiving the request body. So Progress<Outcome<IncomingResponse, Error>> is really no good.

As for download progress I thought it should be deduced from the reading of the response body Source, so I am not sure if we need/want a separate observer, unless the response body is given as a complete Chunk.

https://github.com/haxetink/tink_http/blob/f7ccf01f4b4e46eed9f448406a3d853a3fa8d72a/src/tink/http/Fetch.hx#L133-L176

kevinresol avatar Nov 27 '20 05:11 kevinresol