async-http-client: Cannot cancel inside delegate

To cancel an in-progress request, the user can call cancel() on the HTTPTask.

However, if they are doing streaming there is no way to cancel from inside their HTTPResponseDelegate.

public protocol HTTPResponseDelegate : class {
    associatedtype Response

    func didTransmitRequestBody()
    func didReceiveHead(_ head: HTTPResponseHead)
    func didReceivePart(_ buffer: ByteBuffer)
    func didReceiveError(_ error: Error)
    func didFinishRequest() throws -> Response
}

Functions like didReceivePart() do not have access to the task, so they can’t call cancel(). The delegate can’t store a reference to the task because the delegate is passed in to execute() (for example), which happens before the task is created.

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 24 (12 by maintainers)

Most upvoted comments

Happy to PR this, shouldn’t be too hard (famous last words).

ianpartridge on Apr 11, 2019

I’ve implemented a prototype, it is indeed potentially dangerous. If one returns .continue when .end or error are received, future will never be completed. Only option is to complete promise with some kind of ReturnedNextOnEnd error if we receive .continue on .end, but this is a runtime error, which is not great in already runtime-error-filled environment of async frameworks (with didReceiveEnd you just cannot not provide a result)… One solution here would be to drop promise from HTTPHandler but in this case users of the library will have to provide a promise, which is not great (you cannot just create a promise, you need access to an EventLoop). And it could make code that handles redirects to a different URL origin more complex (I’m not yet sure about that, will test it today). Having state is also not super convenient with this type of functional consumer. With delegate its easier to re-use due to state encapsulation, but in case of a function, you have to have state be captured by a closure, and this is messy, imho, especially if you have more than one variable to capture. One can create a class for that state, but by this point you already have a class, so why not just conform to a protocol and use class as a delegate instead. I will experiment with dropping promise from HTTPHandler to see if it has potential. Also, I will experiment with having only one function in a protocol (similar to RxSwift observer).

artemredkin on Apr 14, 2019

I’ll implement it today-tomorrow and will play with failure modes to see what we can do here

artemredkin on Apr 12, 2019