[swift-server-dev] HTTP API v0.1.0

[Johannes Weiß]

Hi,

> On 4 Oct 2017, at 6:55 pm, Vladimir.S via swift-server-dev <swift-server-dev at swift.org> wrote:
> 
> On 02.10.2017 19:30, Chris Bailey via swift-server-dev wrote:
>> We spent some time updating and validating the HTTP API, along with creating a full set of API docs etc. In order to make it easier for the wider community to build applications that use it to test out the API and provide feedback, I've just tagged v0.1.0.
> 
> In addition to my previous message, I'd like to raise some concrete questions. I understand that most likely they were answered at some point of time during the discussions. But currently there is 0.1.0 version, some checkpoint, and so it attracts more attention of developers who did not participate yet in discussions and I think it is worth to summarize answers in some reply. So :
> 
> (I based my questions on examples of 'echo' and 'hello world' on https://github.com/swift-server/http and on documentation on https://swift-server.github.io/http/ )
> 
> 1. What about change of writeHeader()/writeBody() to write() ? I see that PR was raised, but 0.1.0 is still uses the first form.
> 
> 2. At the moment, when we enter into echo()/hello(), is _body_ of HTTP request already *downloaded*(received from socket) or not?
> I assume it is not downloaded, and this is why we need this HTTPBodyProcessing return value, right?
> What is the logic/strategy of reading a very large bodies of HTTP request from socket? Via chunks in HTTPBodyProcessing?

technically this is an implementation detail. However every sensible implementation will be asynchronous and will only buffer a bounded number of bytes per request. Therefore, for implementations that satisfy this constraint, the body will not necessarily be downloaded in full at the time you enter `echo`.

That's the reason you're supposed to return a closure as the library can call it whenever there's new data available.

And the `finishedProcessing` callback tells the library that you're finished processing the last chunk. That's used to establish back-pressure to the client. In other words if the client sends data quicker than the server can process it the library will need to stop reading after its bounded buffer is exhausted. That will then fill up the receive buffer in the kernel which will then use TCP flow control to communicate to the client that it should stop sending more.


> 3. HTTPRequest is described as "A structure representing the headers from a HTTP request, without the body of the request.". If it is so, why not HTTPRequestHeaders, or HTTPRequestInfo, or HTTPRequestDescription etc ?
> IMO HTTPRequest mentally includes all parts of request, including content. I was very confused by the name when trying to understand the examples.

Even though I named the original HTTPRequest I agree with you. It should be HTTPRequestHead or something I think.


> 4. Returning of HTTPBodyProcessing from handler function.
> 
> 4.a. If the body 'ignored', does this mean that the body content of the request will not be read from socket at all?
> I.e. what is the purpose and logic behind .ignored?

it will need to be read but discarded as efficiently as the library can. If you don't read it you'll break keep-alive.


> 4.b. As was said, HTTPBodyProcessing is long and non-meaning name. First it is not clear *which* body it will process and why we need this process at all and why some processing closure is a return of our handler function.

Very happy to change the name, maybe HTTPRequestBodyProcessing.

The reason that we return it from the handler is that at this point the library will need to know what to do with the body because the client will not start sending it (if there's a body). If you have an idea where to better put it, please suggest something.


> Was it discussed(and if so, why declined) to implement this in some another form, not as result value?
> For example:
> func echo(http: HTTPContext, request: HTTPRequest, response: HTTPResponse) {
> 	response.writeHeader(status: .ok)
> 	http.serve { (chunk, stop) in ..
> 		...
> 	}
> }

that's also a viable API, there's slightly more room for programmer error here as the programmer isn't forced to do anything with the `HTTPContext`.


> 5. HTTPResponseWriter ('response' variable) has a methods to write data. But for reading the data we use HTTPBodyProcessing, for which processing closure returned as associated value for enum instance.
> Did you think about introducing of a HTTPRequestReader, which encapsulates Request details(request's headers) and methods to read the body?
> Then, we probably can have a very nice and Swifty handler code with clear naming and purpose :
> func echo(request: HTTPRequestReader, response: HTTPResponseWriter) {
> 	response.writeHeader(status: .ok)
> 	request.read { chunk in
> 		switch chunk {
>        	case .chunk(let data):
>            		response.writeBody(data) { result in
> 				switch result {
>                		case .ok:
> 					request.next()
> 				case .error:
> 					response.abort()
> 					request.abort()
> 				}
>            		}
>        	case .end:
>            		response.done()
>        	default:            		
>            		response.abort()
> 			request.abort()
> 	}
> }

again more room for error for questionable benefit. But please note that this is supposed to be a low-level HTTP API. If you prefer what you proposed above, there's absolutely nothing stopping you from wrapping the low-level API and exposing it like you do.


> and
> func hello(request: HTTPRequestReader, response: HTTPResponseWriter) {
> 	response.writeHeader(status: .ok)
> 	response.writeBody("Hello, World!")
> 	response.done()
> 
> 	request.discard()
> }
> , then current HTTPRequest instance could be accessed via request.info property or similar.
> The logic behind the scene could be the same : check if request contains a closure assigned in read() method and then use it.
> In client, this will be RequestWriter and ResponseReader accordingly.
> 
> 6. HTTPBodyChunk has .failed case. Why not .error like in Result in completion handler. I think they should be the same for better uniform of code.

sure, I'm personally not fussed about the names. But I agree we should be consistent.


> 7. HTTPResponseWriter.done() and HTTPBodyChunk.end. I believe it will be nice to call them both the same(done or end) for 

well, `done` is an action (has side-effects) and `.end` is a value that tell's you this is the end. They're not quite the same but I see your point. Not too fussed about the names.


> symmetry, so we can have in code:
> switch chunk {
> case .done:
> 	response.done()
> ..
> or
> switch chunk {
> case .end:
> 	response.end()
> ..

definitely makes sense :)


> 8. Why we need convenience functions that just hides completion handler and so make it easy to not process the .error case in Result of completion handler? (I.e. make it easy to skip error checking). Shouldn't they at least trow exceptions?
> If I understand correctly, the 'hello world' example should looks at least like this:
> func hello(request: HTTPRequest, response: HTTPResponseWriter ) -> HTTPBodyProcessing {
> 	try? response.writeHeader(status: .ok)
> 	try? response.writeBody("Hello, World!")
> 	try? response.done()
> 
> 	return .discardBody
> }

you can't throw here as we only asynchronously know if `writeWhatever` and `done` succeeded.


> 9. write*(..) functions in HTTPResponseWriter. Are they writing directly to socket or they are buffered?

implementation detail of the library.


> 10. What is the purpose of HTTPResponseWriter.done() function?

how else do you tell the library that you're done writing the body? It could be (and will often be) chunked encoding so there's no way the library could know. Also the result of the `done` function really tells you if the overall thing was successfully written and sent.

For many application (most that don't stream data for very long) it's useful to ignore the result of each write and only care about the overall result (which you get from `done`). For other applications the `write` callbacks are crucial. Imagine a long-poll server where the client might hold a connection for hours. The server really must know if a write failed as it then wants to stop everything related to that connection and tear it down.

-- Johannes


> Thank you for your time and for answers.
> Vladimir.
> 
>> You can see that here:
>>         HTTP 0.1.0 https://github.com/swift-server/http/tree/0.1.0
>> I've also submitted a PR to add the HTTP API into the set servers/frameworks being tested as part of the "Which is the fastest?" benchmark.
>> In parallel, Gelareh Taban has been working on adding providing a TLS API, and HTTPS support to the HTTP project. You can see her incubator work here:
>>         HTTPS incubator: https://github.com/gtaban/http
>>         TLS incubator:        https://github.com/gtaban/TLSService.git <https://github.com/gtaban/TLSService.git>
>> The aim is to validate and include the TLS work in v0.2.0, along with feedback from the wider community on the existing API surface, fixing bugs, and working on implementing outstanding items like trailer support, backpressure, etc.
>> Chris
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with number 741598.
>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>> _______________________________________________
>> swift-server-dev mailing list
>> swift-server-dev at swift.org
>> https://lists.swift.org/mailman/listinfo/swift-server-dev
> _______________________________________________
> swift-server-dev mailing list
> swift-server-dev at swift.org
> https://lists.swift.org/mailman/listinfo/swift-server-dev