Skip to content

Commit 2189eb6

Browse files
committed
Code review
1 parent 1acdb0c commit 2189eb6

File tree

4 files changed

+40
-7
lines changed

4 files changed

+40
-7
lines changed

Diff for: Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine+Demand.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ extension HTTPRequestStateMachine {
5050
self.state = .modifying
5151
buffer.append(body)
5252
self.state = .waitingForBytes(buffer)
53-
53+
5454
case .modifying:
5555
preconditionFailure("Invalid state: \(self.state)")
5656

Diff for: Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift

+18-6
Original file line numberDiff line numberDiff line change
@@ -523,19 +523,31 @@ struct HTTPRequestStateMachine {
523523
where head.status.code < 300:
524524

525525
return self.avoidingStateMachineCoW { state -> Action in
526-
let (remainingBuffer, _) = responseStreamState.end()
527-
state = .running(
528-
.streaming(expectedBodyLength: expectedBodyLength, sentBodyBytes: sentBodyBytes, producer: producerState),
529-
.endReceived
530-
)
531-
return .forwardResponseBodyParts(remainingBuffer)
526+
let (remainingBuffer, connectionAction) = responseStreamState.end()
527+
switch connectionAction {
528+
case .none:
529+
state = .running(
530+
.streaming(expectedBodyLength: expectedBodyLength, sentBodyBytes: sentBodyBytes, producer: producerState),
531+
.endReceived
532+
)
533+
return .forwardResponseBodyParts(remainingBuffer)
534+
case .close:
535+
// If we receive a `.close` as a connectionAction from the responseStreamState
536+
// this means, that the response end was signaled by a connection close. Since
537+
// the request is still uploading, we will not be able to finish the upload. For
538+
// this reason we can fail the request here.
539+
state = .failed(HTTPClientError.remoteConnectionClosed)
540+
return .failRequest(HTTPClientError.remoteConnectionClosed, .close)
541+
}
532542
}
533543

534544
case .running(.streaming(_, _, let producerState), .receivingBody(let head, var responseStreamState)):
535545
assert(head.status.code >= 300)
536546
assert(producerState == .paused, "Expected to have paused the request body stream, when the head was received. Invalid state: \(self.state)")
537547

538548
return self.avoidingStateMachineCoW { state -> Action in
549+
// We can ignore the connectionAction from the responseStreamState, since the
550+
// connection should be closed anyway.
539551
let (remainingBuffer, _) = responseStreamState.end()
540552
state = .finished
541553
return .succeedRequest(.close, remainingBuffer)

Diff for: Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ extension HTTPRequestStateMachineTests {
5252
("testErrorWhileRunningARequestClosesTheStream", testErrorWhileRunningARequestClosesTheStream),
5353
("testCanReadHTTP1_0ResponseWithoutBody", testCanReadHTTP1_0ResponseWithoutBody),
5454
("testCanReadHTTP1_0ResponseWithBody", testCanReadHTTP1_0ResponseWithBody),
55+
("testFailHTTP1_0RequestThatIsStillUploading", testFailHTTP1_0RequestThatIsStillUploading),
5556
]
5657
}
5758
}

Diff for: Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift

+20
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,26 @@ class HTTPRequestStateMachineTests: XCTestCase {
485485
XCTAssertEqual(state.channelRead(.end(nil)), .succeedRequest(.close, [body]))
486486
XCTAssertEqual(state.channelInactive(), .wait)
487487
}
488+
489+
func testFailHTTP1_0RequestThatIsStillUploading() {
490+
var state = HTTPRequestStateMachine(isChannelWritable: true)
491+
let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/")
492+
let metadata = RequestFramingMetadata(connectionClose: false, body: .stream)
493+
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true))
494+
495+
let part1: ByteBuffer = .init(string: "foo")
496+
XCTAssertEqual(state.requestStreamPartReceived(.byteBuffer(part1)), .sendBodyPart(.byteBuffer(part1)))
497+
let responseHead = HTTPResponseHead(version: .http1_0, status: .ok)
498+
let body = ByteBuffer(string: "foo bar")
499+
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
500+
XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait)
501+
XCTAssertEqual(state.channelReadComplete(), .wait)
502+
XCTAssertEqual(state.read(), .read)
503+
XCTAssertEqual(state.channelReadComplete(), .wait)
504+
XCTAssertEqual(state.channelRead(.body(body)), .wait)
505+
XCTAssertEqual(state.channelRead(.end(nil)), .failRequest(HTTPClientError.remoteConnectionClosed, .close))
506+
XCTAssertEqual(state.channelInactive(), .wait)
507+
}
488508
}
489509

490510
extension HTTPRequestStateMachine.Action: Equatable {

0 commit comments

Comments
 (0)