unclear docs: How to signal to StreamWriter that body is done?

[weissi]

StreamWriter's complete docs are

        /// Chunk provider.
        public struct StreamWriter {
            /// Write data to server.
            ///
            /// - parameters:
            ///     - data: `IOData` to write.
            public func write(_ data: IOData) -> EventLoopFuture<Void>
        }

I understand how I can provide chunks: by calling write but how do I tell the StreamWriter that I'm done sending request body bytes?

[adam-fowler]

@weissi You provide it with a length at the start and once you've supplied that data, it completes. I'm not a great fan of this interface. It is really confusing what is happening. Here is my attempt at implementing one https://github.com/swift-aws/aws-sdk-swift-core/blob/bdbac3768398b8387e2a81be193b6c42da75979e/Sources/AWSSDKSwiftCore/HTTP/AsyncHTTPClient.swift#L23

[weissi]

@adam-fowler but how do I do streaming? Ie. if I don't know the content length? Then chunked encoding should be selected...

[weissi]

CC @artemredkin

[adam-fowler]

@adam-fowler but how do I do streaming? Ie. if I don't know the content length? Then chunked encoding should be selected...

Yes that's a good question. That would be very useful.

[adam-fowler]

@weissi is this something that could be resolved for 1.2. It doesn't look like that complex a thing to do. Return an empty ByteBuffer() to indicate end or something like that and add a chunked header.

[adam-fowler]

Oh shit just thought this might be a breaking change. Although you could just add another version of the stream function.

[artemredkin]

@weissi answering your question, you just return succeeded future from write method:

.stream() { writer in
    writer.write()
    writer.write()
    ...
    writer.write()
    return eventLoop.makeSucceededFuture(())
}

I'm not a fan of this interface, it was designed mainly around how NonBlockingFileIO works...

[artemredkin]

Obviously, this example has absolutely no backpressure, so another alternative will be something like:

.stream() { writer in
    writer.write().flatMap {
        writer.write()
    }
}

[adam-fowler]

Sorry @artem I completely missed that. For some reason I had got it into my head you were required to pass in a content length

[artemredkin]

No worries, our docs in this area are not good, and API is not very clear at all. I'll try to improve streaming docs/API's after 1.2.0 release.

[weissi]

@artemredkin Hmm, I don't get it. A user gets handed a StreamWriter, they then call .write for every bit of data they have, how do they communicate that that's everything? So that there's nothing more coming?

[artemredkin]

user sets a body and passes it a closure, that has a singature:

(StreamWriter) -> EventLoopFuture<Void>

as soon as this future completes, we send .end

[adam-fowler]

@weissi I found it is a little confusing initially. I have a working version here, where you supply a stream function that returns ByteBuffers until you have finished streaming or have hit a specified size. https://github.com/swift-aws/aws-sdk-swift-core/blob/16cce9fe9fbc0f5949fb48a6fd7dce90b3645357/Sources/AWSSDKSwiftCore/HTTP/AsyncHTTPClient.swift#L78

[weissi]

@artemredkin oooh, so the user's meant to allocate a promise and finish it when they're done with the body? That seems super dangerous, they need to then also detect all the possible error cases and funnel them into this promise, or else they'll crash.

I think we should create a new API that just gives you a done method on StreamWriter, no?

Or was there a use-case that this was designed for?

[adam-fowler]

@weissi they need to return a future. I allocated the promise so I wouldnt have to chain all the futures

[artemredkin]

I built it around how NonBlockingFileIO works, readChunked specifically

[weissi]

@adam-fowler yeah but to return a future that they can fulfil when they're done, they need to allocate a promise that they hold elsewhere.

So basically users need to keep track of two things:

• a promise (that they MUST fulfil in all error cases too)
• a stream writer

Confusingly, the feed new data through the stream writer but send end through the promise.

[weissi]

@artemredkin readChunked is an API that gives you data, ie. a source. write is an API that you give data to, ie. a sink. They aren't related, are they?

The future you return in readChunked is to propagate backpressure and I think the API makes sense. The future you return in the stream writer providing closure is to indicate the end of a HTTP request body, I think that's quite confusing, no?

[artemredkin]

it totally is super confusing, yeah, it was like a mirror API, so it turned out quite strange

[adam-fowler]

Yeah it is confusing. Took me a while to work out how to get it working. Personally I would prefer, an API where a stream closure returning an EventLoopFuture<ByteBuffer> is called repeatedly until it returns an empty ByteBuffer.

[artemredkin]

How about a an enum:

enum Part {
    case next(ByteBuffer)
    case end
}

wdyt?

[adam-fowler]

So it returns an EventLoopFuture<Part>? Yeah that'd work

[weissi]

@artemredkin we already got the StreamWriter type, why not just use methods?

streamWriter.write(...)
[...]
streamWriter.end(...)

I mean I'm happy with an enum too but then the API would become something like

streamWriter.do(.next(buffer))
[...]
streamWriter.do(.end(...))

I'd just use methods but don't mind. Either is a big step up from where we are right now.

One other thing that I found very inconvenient was that I had to wait for a callback to get the StreamWriter. Why can't I get it synchronously off the Task?

[artemredkin]

Methods seem a nicer API, moving to 2.0.0 to introduce a new API

[weissi]

@artemredkin we need to do this before 2.0.0. We can do this with deprecations, pretty sure.

[mredig]

@weissi answering your question, you just return succeeded future from write method:

.stream() { writer in
writer.write()
writer.write()
...
writer.write()
return eventLoop.makeSucceededFuture(())
}
I'm not a fan of this interface, it was designed mainly around how NonBlockingFileIO works...

I hope this is the "right" way to do this, because that's what I did.

I read through the rest of this convo and it appears that there's a more intuitive approach on the horizon, but in the meantime can we get this example (or whatever would be best) in the docs?

[Lukasa]

If at all possible, you can use the async/await APIs and their new Body type. That has a much clearer interface.

[mredig]

If at all possible, you can use the async/await APIs and their new Body type. That has a much clearer interface.

looks like I can't because of #819 😢