-
Notifications
You must be signed in to change notification settings - Fork 287
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
Add support for HTTP Streaming #2140
base: v2
Are you sure you want to change the base?
Conversation
Package Changes Through 3f9f336No changes. Add a change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, could you also add a change file in .changes
directory?
statusText | ||
// If the promise fails, make sure the stream is closed | ||
readPromise.catch((e) => { | ||
console.error('error reading body', e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need to console.error here? won't controller.error be enough?
let res = Arc::into_inner(res).unwrap().0; | ||
Ok(tauri::ipc::Response::new(res.bytes().await?.to_vec())) | ||
let mut stream = res.bytes_stream(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we do the same streaming behavior using Response::chunk
method, instead of pulling another crate to iterate over the stream?
This PR changes the internals of the fetch implementation such that
fetch_read_body
uses theChannel
API to stream http responses instead of waiting for the entire response to finish and then returning the final ArrayBuffer.Two things to note:
The
Channel<&[u8]>
type is serializing as anumber[]
instead ofArrayBuffer
orUint8Array
. I'm not certain that the Channel supports serializing asArrayBuffer
s? I think this could be a possible performance improvement.The
Channel
API implementation does not guarantee that all events are received by the time theinvoke
promise is resolved. This is unfortunate, but I worked around it by returning an empty array over theChannel
which is used to close theReadableStream
. I think this design works alright, but maybe there's a cleaner way to do this.Fixes #2129