From e097940a498f08db75f5c6c52f2127b407bdb3f0 Mon Sep 17 00:00:00 2001
From: David Nadoba <d_nadoba@apple.com>
Date: Wed, 24 Nov 2021 07:07:27 +0100
Subject: [PATCH 1/8] fix bodyLengthMissmatch error handling
---
.../HTTPRequestStateMachine.swift | 6 ++---
.../HTTPRequestStateMachineTests+XCTest.swift | 2 ++
.../HTTPRequestStateMachineTests.swift | 24 +++++++++++++++++++
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
index 2d6ad5dd8..a3a9c38e0 100644
--- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
+++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
@@ -198,14 +198,14 @@ struct HTTPRequestStateMachine {
self.state = .failed(error)
return .failRequest(error, .none)
- case .running(.streaming, .waitingForHead),
+ case .running(.streaming, .waitingForHead) where error as? NIOSSLError == .uncleanShutdown,
.running(.endSent, .waitingForHead) where error as? NIOSSLError == .uncleanShutdown:
// if we received a NIOSSL.uncleanShutdown before we got an answer we should handle
// this like a normal connection close. We will receive a call to channelInactive after
// this error.
return .wait
- case .running(.streaming, .receivingBody(let responseHead, _)),
+ case .running(.streaming, .receivingBody(let responseHead, _)) where error as? NIOSSLError == .uncleanShutdown,
.running(.endSent, .receivingBody(let responseHead, _)) where error as? NIOSSLError == .uncleanShutdown:
// This code is only reachable for request and responses, which we expect to have a body.
// We depend on logic from the HTTPResponseDecoder here. The decoder will emit an
@@ -270,7 +270,7 @@ struct HTTPRequestStateMachine {
if let expected = expectedBodyLength, sentBodyBytes + part.readableBytes > expected {
let error = HTTPClientError.bodyLengthMismatch
-
+ self.state = .failed(error)
return .failRequest(error, .close)
}
diff --git a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift
index 3dd6c7b30..b54865fd8 100644
--- a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests+XCTest.swift
@@ -54,6 +54,8 @@ extension HTTPRequestStateMachineTests {
("testCanReadHTTP1_0ResponseWithBody", testCanReadHTTP1_0ResponseWithBody),
("testFailHTTP1_0RequestThatIsStillUploading", testFailHTTP1_0RequestThatIsStillUploading),
("testFailHTTP1RequestWithoutContentLengthWithNIOSSLErrorUncleanShutdown", testFailHTTP1RequestWithoutContentLengthWithNIOSSLErrorUncleanShutdown),
+ ("testNIOSSLErrorUncleanShutdownShouldBeTreatedAsRemoteConnectionCloseWhileInWaitingForHeadState", testNIOSSLErrorUncleanShutdownShouldBeTreatedAsRemoteConnectionCloseWhileInWaitingForHeadState),
+ ("testArbitraryErrorShouldBeTreatedAsARequestFailureWhileInWaitingForHeadState", testArbitraryErrorShouldBeTreatedAsARequestFailureWhileInWaitingForHeadState),
("testFailHTTP1RequestWithContentLengthWithNIOSSLErrorUncleanShutdownButIgnoreIt", testFailHTTP1RequestWithContentLengthWithNIOSSLErrorUncleanShutdownButIgnoreIt),
("testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForDemand", testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForDemand),
("testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForRead", testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForRead),
diff --git a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
index 3e92dc87f..edbd3a002 100644
--- a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
@@ -82,6 +82,9 @@ class HTTPRequestStateMachineTests: XCTestCase {
}
XCTAssertEqual(error as? HTTPClientError, .bodyLengthMismatch)
+
+ // if another error happens the new one is ignored
+ XCTAssertEqual(state.errorHappened(HTTPClientError.remoteConnectionClosed), .wait)
}
func testPOSTContentLengthIsTooShort() {
@@ -522,6 +525,27 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelInactive(), .wait)
}
+ func testNIOSSLErrorUncleanShutdownShouldBeTreatedAsRemoteConnectionCloseWhileInWaitingForHeadState() {
+ var state = HTTPRequestStateMachine(isChannelWritable: true)
+ let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
+ let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
+ XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
+
+ XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .wait)
+ XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ }
+
+ func testArbitraryErrorShouldBeTreatedAsARequestFailureWhileInWaitingForHeadState() {
+ struct ArbitraryError: Error {}
+ var state = HTTPRequestStateMachine(isChannelWritable: true)
+ let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
+ let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
+ XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
+
+ XCTAssertEqual(state.errorHappened(ArbitraryError()), .failRequest(ArbitraryError(), .close))
+ XCTAssertEqual(state.channelInactive(), .wait)
+ }
+
func testFailHTTP1RequestWithContentLengthWithNIOSSLErrorUncleanShutdownButIgnoreIt() {
var state = HTTPRequestStateMachine(isChannelWritable: true)
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
From 3a6a256f6ae105ba7b0f4fb716f559380197bc88 Mon Sep 17 00:00:00 2001
From: David Nadoba <d_nadoba@apple.com>
Date: Wed, 24 Nov 2021 10:21:19 +0100
Subject: [PATCH 2/8] check error through pattern matching
because `HTTPRequestStateMachine.Action` Equatable conformance ignores the error completly
---
.../HTTPRequestStateMachineTests.swift | 87 +++++++++++++++----
1 file changed, 69 insertions(+), 18 deletions(-)
diff --git a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
index edbd3a002..5df644cfa 100644
--- a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
@@ -209,7 +209,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
let part1 = IOData.byteBuffer(ByteBuffer(bytes: 4...7))
XCTAssertEqual(state.requestStreamPartReceived(part1), .sendBodyPart(part1))
- XCTAssertEqual(state.requestStreamFinished(), .failRequest(HTTPClientError.bodyLengthMismatch, .close))
+ guard case .failRequest(let error, .close) = state.requestStreamFinished() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .bodyLengthMismatch)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -227,7 +230,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
let part1 = IOData.byteBuffer(ByteBuffer(bytes: 4...7))
XCTAssertEqual(state.requestStreamPartReceived(part1), .sendBodyPart(part1))
- XCTAssertEqual(state.requestStreamFinished(), .failRequest(HTTPClientError.bodyLengthMismatch, .close))
+ guard case .failRequest(let error, .close) = state.requestStreamFinished() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .bodyLengthMismatch)
XCTAssertEqual(state.channelRead(.end(nil)), .wait)
}
@@ -252,7 +258,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ guard case .failRequest(let error, .none) = state.channelInactive() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
}
func testResponseReadingWithBackpressure() {
@@ -342,7 +351,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
func testCancellingARequestInStateInitializedKeepsTheConnectionAlive() {
var state = HTTPRequestStateMachine(isChannelWritable: false)
- XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .none))
+ guard case .failRequest(let error, .none) = state.requestCancelled() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .cancelled)
}
func testCancellingARequestBeforeBeingSendKeepsTheConnectionAlive() {
@@ -350,7 +362,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .wait)
- XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .none))
+ guard case .failRequest(let error, .none) = state.requestCancelled() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .cancelled)
}
func testConnectionBecomesWritableBeforeFirstRequest() {
@@ -376,7 +391,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
- XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .close))
+ guard case .failRequest(let error, .close) = state.requestCancelled() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .cancelled)
}
func testRemoteSuddenlyClosesTheConnection() {
@@ -384,7 +402,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/", headers: .init([("content-length", "4")]))
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true))
- XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.remoteConnectionClosed, .close))
+ guard case .failRequest(let error, .close) = state.requestCancelled() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .cancelled)
XCTAssertEqual(state.requestStreamPartReceived(.byteBuffer(.init(bytes: 1...3))), .wait)
}
@@ -398,7 +419,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
let part0 = ByteBuffer(bytes: 0...3)
XCTAssertEqual(state.channelRead(.body(part0)), .wait)
- XCTAssertEqual(state.idleReadTimeoutTriggered(), .failRequest(HTTPClientError.readTimeout, .close))
+ guard case .failRequest(let error, .close) = state.idleReadTimeoutTriggered() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .readTimeout)
XCTAssertEqual(state.channelRead(.body(ByteBuffer(bytes: 4...7))), .wait)
XCTAssertEqual(state.channelRead(.body(ByteBuffer(bytes: 8...11))), .wait)
XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait)
@@ -451,7 +475,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
- XCTAssertEqual(state.errorHappened(HTTPParserError.invalidChunkSize), .failRequest(HTTPParserError.invalidChunkSize, .close))
+ guard case .failRequest(let error, .close) = state.errorHappened(HTTPParserError.invalidChunkSize) else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPParserError, .invalidChunkSize)
XCTAssertEqual(state.requestCancelled(), .wait, "A cancellation that happens to late is ignored")
}
@@ -505,7 +532,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.read(), .read)
XCTAssertEqual(state.channelReadComplete(), .wait)
XCTAssertEqual(state.channelRead(.body(body)), .wait)
- XCTAssertEqual(state.channelRead(.end(nil)), .failRequest(HTTPClientError.remoteConnectionClosed, .close))
+ guard case .failRequest(let error, .close) = state.channelRead(.end(nil)) else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -520,7 +550,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait)
XCTAssertEqual(state.channelRead(.body(body)), .wait)
- XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .failRequest(NIOSSLError.uncleanShutdown, .close))
+ guard case .failRequest(let error, .close) = state.errorHappened(NIOSSLError.uncleanShutdown) else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? NIOSSLError, .uncleanShutdown)
XCTAssertEqual(state.channelRead(.end(nil)), .wait)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -532,7 +565,11 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+
+ guard case .failRequest(let error, .none) = state.channelInactive() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
}
func testArbitraryErrorShouldBeTreatedAsARequestFailureWhileInWaitingForHeadState() {
@@ -541,8 +578,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
-
- XCTAssertEqual(state.errorHappened(ArbitraryError()), .failRequest(ArbitraryError(), .close))
+ guard case .failRequest(let error, .close) = state.errorHappened(ArbitraryError()) else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertTrue(error is ArbitraryError)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -560,7 +599,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(body)), .wait)
XCTAssertEqual(state.channelReadComplete(), .forwardResponseBodyParts([body]))
XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .wait)
- XCTAssertEqual(state.errorHappened(HTTPParserError.invalidEOFState), .failRequest(HTTPParserError.invalidEOFState, .close))
+ guard case .failRequest(let error, .close) = state.errorHappened(HTTPParserError.invalidEOFState) else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPParserError, .invalidEOFState)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -582,7 +624,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: " baz lightyear"))), .wait)
XCTAssertEqual(state.channelReadComplete(), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ guard case .failRequest(let error, .none) = state.channelInactive() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
}
func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForRead() {
@@ -603,7 +648,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: " baz lightyear"))), .wait)
XCTAssertEqual(state.channelReadComplete(), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ guard case .failRequest(let error, .none) = state.channelInactive() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
}
func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForReadAndDemand() {
@@ -623,7 +671,10 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: " baz lightyear"))), .wait)
XCTAssertEqual(state.channelReadComplete(), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ guard case .failRequest(let error, .none) = state.channelInactive() else {
+ return XCTFail("unexpected action")
+ }
+ XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
}
func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForReadAndDemandMultipleTimes() {
From 0d8930ccf7e830254478e51a8365636033919e9e Mon Sep 17 00:00:00 2001
From: David Nadoba <d_nadoba@apple.com>
Date: Wed, 24 Nov 2021 11:02:34 +0100
Subject: [PATCH 3/8] Revert "check error through pattern matching"
This reverts commit 3a6a256f6ae105ba7b0f4fb716f559380197bc88.
---
.../HTTPRequestStateMachineTests.swift | 87 ++++---------------
1 file changed, 18 insertions(+), 69 deletions(-)
diff --git a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
index 5df644cfa..edbd3a002 100644
--- a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
@@ -209,10 +209,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let part1 = IOData.byteBuffer(ByteBuffer(bytes: 4...7))
XCTAssertEqual(state.requestStreamPartReceived(part1), .sendBodyPart(part1))
- guard case .failRequest(let error, .close) = state.requestStreamFinished() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .bodyLengthMismatch)
+ XCTAssertEqual(state.requestStreamFinished(), .failRequest(HTTPClientError.bodyLengthMismatch, .close))
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -230,10 +227,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let part1 = IOData.byteBuffer(ByteBuffer(bytes: 4...7))
XCTAssertEqual(state.requestStreamPartReceived(part1), .sendBodyPart(part1))
- guard case .failRequest(let error, .close) = state.requestStreamFinished() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .bodyLengthMismatch)
+ XCTAssertEqual(state.requestStreamFinished(), .failRequest(HTTPClientError.bodyLengthMismatch, .close))
XCTAssertEqual(state.channelRead(.end(nil)), .wait)
}
@@ -258,10 +252,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .wait)
- guard case .failRequest(let error, .none) = state.channelInactive() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
+ XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
}
func testResponseReadingWithBackpressure() {
@@ -351,10 +342,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
func testCancellingARequestInStateInitializedKeepsTheConnectionAlive() {
var state = HTTPRequestStateMachine(isChannelWritable: false)
- guard case .failRequest(let error, .none) = state.requestCancelled() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .cancelled)
+ XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .none))
}
func testCancellingARequestBeforeBeingSendKeepsTheConnectionAlive() {
@@ -362,10 +350,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .wait)
- guard case .failRequest(let error, .none) = state.requestCancelled() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .cancelled)
+ XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .none))
}
func testConnectionBecomesWritableBeforeFirstRequest() {
@@ -391,10 +376,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
- guard case .failRequest(let error, .close) = state.requestCancelled() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .cancelled)
+ XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .close))
}
func testRemoteSuddenlyClosesTheConnection() {
@@ -402,10 +384,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/", headers: .init([("content-length", "4")]))
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true))
- guard case .failRequest(let error, .close) = state.requestCancelled() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .cancelled)
+ XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.remoteConnectionClosed, .close))
XCTAssertEqual(state.requestStreamPartReceived(.byteBuffer(.init(bytes: 1...3))), .wait)
}
@@ -419,10 +398,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
let part0 = ByteBuffer(bytes: 0...3)
XCTAssertEqual(state.channelRead(.body(part0)), .wait)
- guard case .failRequest(let error, .close) = state.idleReadTimeoutTriggered() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .readTimeout)
+ XCTAssertEqual(state.idleReadTimeoutTriggered(), .failRequest(HTTPClientError.readTimeout, .close))
XCTAssertEqual(state.channelRead(.body(ByteBuffer(bytes: 4...7))), .wait)
XCTAssertEqual(state.channelRead(.body(ByteBuffer(bytes: 8...11))), .wait)
XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait)
@@ -475,10 +451,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
- guard case .failRequest(let error, .close) = state.errorHappened(HTTPParserError.invalidChunkSize) else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPParserError, .invalidChunkSize)
+ XCTAssertEqual(state.errorHappened(HTTPParserError.invalidChunkSize), .failRequest(HTTPParserError.invalidChunkSize, .close))
XCTAssertEqual(state.requestCancelled(), .wait, "A cancellation that happens to late is ignored")
}
@@ -532,10 +505,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.read(), .read)
XCTAssertEqual(state.channelReadComplete(), .wait)
XCTAssertEqual(state.channelRead(.body(body)), .wait)
- guard case .failRequest(let error, .close) = state.channelRead(.end(nil)) else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
+ XCTAssertEqual(state.channelRead(.end(nil)), .failRequest(HTTPClientError.remoteConnectionClosed, .close))
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -550,10 +520,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait)
XCTAssertEqual(state.channelRead(.body(body)), .wait)
- guard case .failRequest(let error, .close) = state.errorHappened(NIOSSLError.uncleanShutdown) else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? NIOSSLError, .uncleanShutdown)
+ XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .failRequest(NIOSSLError.uncleanShutdown, .close))
XCTAssertEqual(state.channelRead(.end(nil)), .wait)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -565,11 +532,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .wait)
-
- guard case .failRequest(let error, .none) = state.channelInactive() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
+ XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
}
func testArbitraryErrorShouldBeTreatedAsARequestFailureWhileInWaitingForHeadState() {
@@ -578,10 +541,8 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
- guard case .failRequest(let error, .close) = state.errorHappened(ArbitraryError()) else {
- return XCTFail("unexpected action")
- }
- XCTAssertTrue(error is ArbitraryError)
+
+ XCTAssertEqual(state.errorHappened(ArbitraryError()), .failRequest(ArbitraryError(), .close))
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -599,10 +560,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(body)), .wait)
XCTAssertEqual(state.channelReadComplete(), .forwardResponseBodyParts([body]))
XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .wait)
- guard case .failRequest(let error, .close) = state.errorHappened(HTTPParserError.invalidEOFState) else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPParserError, .invalidEOFState)
+ XCTAssertEqual(state.errorHappened(HTTPParserError.invalidEOFState), .failRequest(HTTPParserError.invalidEOFState, .close))
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -624,10 +582,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: " baz lightyear"))), .wait)
XCTAssertEqual(state.channelReadComplete(), .wait)
- guard case .failRequest(let error, .none) = state.channelInactive() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
+ XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
}
func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForRead() {
@@ -648,10 +603,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: " baz lightyear"))), .wait)
XCTAssertEqual(state.channelReadComplete(), .wait)
- guard case .failRequest(let error, .none) = state.channelInactive() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
+ XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
}
func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForReadAndDemand() {
@@ -671,10 +623,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: " baz lightyear"))), .wait)
XCTAssertEqual(state.channelReadComplete(), .wait)
- guard case .failRequest(let error, .none) = state.channelInactive() else {
- return XCTFail("unexpected action")
- }
- XCTAssertEqual(error as? HTTPClientError, .remoteConnectionClosed)
+ XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
}
func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForReadAndDemandMultipleTimes() {
From 0bf82398ad776c040de2c62d89f348787a3068ac Mon Sep 17 00:00:00 2001
From: David Nadoba <d_nadoba@apple.com>
Date: Wed, 24 Nov 2021 11:11:25 +0100
Subject: [PATCH 4/8] add and use new assertFailRequest method
---
.../HTTPRequestStateMachineTests.swift | 69 +++++++++++--------
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
index edbd3a002..ab55345c9 100644
--- a/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
+++ b/Tests/AsyncHTTPClientTests/HTTPRequestStateMachineTests.swift
@@ -76,12 +76,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let part1 = IOData.byteBuffer(ByteBuffer(bytes: [0, 1, 2, 3]))
XCTAssertEqual(state.requestStreamPartReceived(part0), .sendBodyPart(part0))
- let failAction = state.requestStreamPartReceived(part1)
- guard case .failRequest(let error, .close) = failAction else {
- return XCTFail("Unexpected action: \(failAction)")
- }
-
- XCTAssertEqual(error as? HTTPClientError, .bodyLengthMismatch)
+ state.requestStreamPartReceived(part1).assertFailRequest(HTTPClientError.bodyLengthMismatch, .close)
// if another error happens the new one is ignored
XCTAssertEqual(state.errorHappened(HTTPClientError.remoteConnectionClosed), .wait)
@@ -95,12 +90,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let part0 = IOData.byteBuffer(ByteBuffer(bytes: [0, 1, 2, 3]))
XCTAssertEqual(state.requestStreamPartReceived(part0), .sendBodyPart(part0))
- let failAction = state.requestStreamFinished()
- guard case .failRequest(let error, .close) = failAction else {
- return XCTFail("Unexpected action: \(failAction)")
- }
-
- XCTAssertEqual(error as? HTTPClientError, .bodyLengthMismatch)
+ state.requestStreamFinished().assertFailRequest(HTTPClientError.bodyLengthMismatch, .close)
}
func testRequestBodyStreamIsCancelledIfServerRespondsWith301() {
@@ -209,7 +199,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let part1 = IOData.byteBuffer(ByteBuffer(bytes: 4...7))
XCTAssertEqual(state.requestStreamPartReceived(part1), .sendBodyPart(part1))
- XCTAssertEqual(state.requestStreamFinished(), .failRequest(HTTPClientError.bodyLengthMismatch, .close))
+ state.requestStreamFinished().assertFailRequest(HTTPClientError.bodyLengthMismatch, .close)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -227,7 +217,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let part1 = IOData.byteBuffer(ByteBuffer(bytes: 4...7))
XCTAssertEqual(state.requestStreamPartReceived(part1), .sendBodyPart(part1))
- XCTAssertEqual(state.requestStreamFinished(), .failRequest(HTTPClientError.bodyLengthMismatch, .close))
+ state.requestStreamFinished().assertFailRequest(HTTPClientError.bodyLengthMismatch, .close)
XCTAssertEqual(state.channelRead(.end(nil)), .wait)
}
@@ -252,7 +242,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ state.channelInactive().assertFailRequest(HTTPClientError.remoteConnectionClosed, .none)
}
func testResponseReadingWithBackpressure() {
@@ -342,7 +332,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
func testCancellingARequestInStateInitializedKeepsTheConnectionAlive() {
var state = HTTPRequestStateMachine(isChannelWritable: false)
- XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .none))
+ state.requestCancelled().assertFailRequest(HTTPClientError.cancelled, .none)
}
func testCancellingARequestBeforeBeingSendKeepsTheConnectionAlive() {
@@ -350,7 +340,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .wait)
- XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .none))
+ state.requestCancelled().assertFailRequest(HTTPClientError.cancelled, .none)
}
func testConnectionBecomesWritableBeforeFirstRequest() {
@@ -376,7 +366,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
- XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.cancelled, .close))
+ state.requestCancelled().assertFailRequest(HTTPClientError.cancelled, .close)
}
func testRemoteSuddenlyClosesTheConnection() {
@@ -384,7 +374,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/", headers: .init([("content-length", "4")]))
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: true))
- XCTAssertEqual(state.requestCancelled(), .failRequest(HTTPClientError.remoteConnectionClosed, .close))
+ state.requestCancelled().assertFailRequest(HTTPClientError.cancelled, .close)
XCTAssertEqual(state.requestStreamPartReceived(.byteBuffer(.init(bytes: 1...3))), .wait)
}
@@ -398,7 +388,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
let part0 = ByteBuffer(bytes: 0...3)
XCTAssertEqual(state.channelRead(.body(part0)), .wait)
- XCTAssertEqual(state.idleReadTimeoutTriggered(), .failRequest(HTTPClientError.readTimeout, .close))
+ state.idleReadTimeoutTriggered().assertFailRequest(HTTPClientError.readTimeout, .close)
XCTAssertEqual(state.channelRead(.body(ByteBuffer(bytes: 4...7))), .wait)
XCTAssertEqual(state.channelRead(.body(ByteBuffer(bytes: 8...11))), .wait)
XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait)
@@ -451,7 +441,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
- XCTAssertEqual(state.errorHappened(HTTPParserError.invalidChunkSize), .failRequest(HTTPParserError.invalidChunkSize, .close))
+ state.errorHappened(HTTPParserError.invalidChunkSize).assertFailRequest(HTTPParserError.invalidChunkSize, .close)
XCTAssertEqual(state.requestCancelled(), .wait, "A cancellation that happens to late is ignored")
}
@@ -505,7 +495,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.read(), .read)
XCTAssertEqual(state.channelReadComplete(), .wait)
XCTAssertEqual(state.channelRead(.body(body)), .wait)
- XCTAssertEqual(state.channelRead(.end(nil)), .failRequest(HTTPClientError.remoteConnectionClosed, .close))
+ state.channelRead(.end(nil)).assertFailRequest(HTTPClientError.remoteConnectionClosed, .close)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -520,7 +510,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.head(responseHead)), .forwardResponseHead(responseHead, pauseRequestBodyStream: false))
XCTAssertEqual(state.demandMoreResponseBodyParts(), .wait)
XCTAssertEqual(state.channelRead(.body(body)), .wait)
- XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .failRequest(NIOSSLError.uncleanShutdown, .close))
+ state.errorHappened(NIOSSLError.uncleanShutdown).assertFailRequest(NIOSSLError.uncleanShutdown, .close)
XCTAssertEqual(state.channelRead(.end(nil)), .wait)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -532,17 +522,17 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ state.channelInactive().assertFailRequest(HTTPClientError.remoteConnectionClosed, .none)
}
func testArbitraryErrorShouldBeTreatedAsARequestFailureWhileInWaitingForHeadState() {
- struct ArbitraryError: Error {}
+ struct ArbitraryError: Error, Equatable {}
var state = HTTPRequestStateMachine(isChannelWritable: true)
let requestHead = HTTPRequestHead(version: .http1_1, method: .GET, uri: "/")
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(0))
XCTAssertEqual(state.startRequest(head: requestHead, metadata: metadata), .sendRequestHead(requestHead, startBody: false))
- XCTAssertEqual(state.errorHappened(ArbitraryError()), .failRequest(ArbitraryError(), .close))
+ state.errorHappened(ArbitraryError()).assertFailRequest(ArbitraryError(), .close)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -560,7 +550,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(body)), .wait)
XCTAssertEqual(state.channelReadComplete(), .forwardResponseBodyParts([body]))
XCTAssertEqual(state.errorHappened(NIOSSLError.uncleanShutdown), .wait)
- XCTAssertEqual(state.errorHappened(HTTPParserError.invalidEOFState), .failRequest(HTTPParserError.invalidEOFState, .close))
+ state.errorHappened(HTTPParserError.invalidEOFState).assertFailRequest(HTTPParserError.invalidEOFState, .close)
XCTAssertEqual(state.channelInactive(), .wait)
}
@@ -582,7 +572,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: " baz lightyear"))), .wait)
XCTAssertEqual(state.channelReadComplete(), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ state.channelInactive().assertFailRequest(HTTPClientError.remoteConnectionClosed, .none)
}
func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForRead() {
@@ -603,7 +593,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: " baz lightyear"))), .wait)
XCTAssertEqual(state.channelReadComplete(), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ state.channelInactive().assertFailRequest(HTTPClientError.remoteConnectionClosed, .none)
}
func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForReadAndDemand() {
@@ -623,7 +613,7 @@ class HTTPRequestStateMachineTests: XCTestCase {
XCTAssertEqual(state.channelRead(.body(ByteBuffer(string: " baz lightyear"))), .wait)
XCTAssertEqual(state.channelReadComplete(), .wait)
- XCTAssertEqual(state.channelInactive(), .failRequest(HTTPClientError.remoteConnectionClosed, .none))
+ state.channelInactive().assertFailRequest(HTTPClientError.remoteConnectionClosed, .none)
}
func testFailHTTPRequestWithContentLengthBecauseOfChannelInactiveWaitingForReadAndDemandMultipleTimes() {
@@ -700,3 +690,22 @@ extension HTTPRequestStateMachine.Action: Equatable {
}
}
}
+
+extension HTTPRequestStateMachine.Action {
+ fileprivate func assertFailRequest<Error>(
+ _ expectedError: Error,
+ _ expectedFinalStreamAction: HTTPRequestStateMachine.Action.FinalStreamAction,
+ file: StaticString = #file,
+ line: UInt = #line
+ ) where Error: Swift.Error & Equatable {
+ guard case .failRequest(let actualError, let actualFinalStreamAction) = self else {
+ return XCTFail("expected .failRequest(\(expectedError), \(expectedFinalStreamAction)) but got \(self)", file: file, line: line)
+ }
+ if let actualError = actualError as? Error {
+ XCTAssertEqual(actualError, expectedError, file: file, line: line)
+ } else {
+ XCTFail("\(actualError) is not equal to \(expectedError)", file: file, line: line)
+ }
+ XCTAssertEqual(actualFinalStreamAction, expectedFinalStreamAction, file: file, line: line)
+ }
+}
From 99aaeecf771c3129959734904f483b4c11cc5be7 Mon Sep 17 00:00:00 2001
From: David Nadoba <d_nadoba@apple.com>
Date: Wed, 24 Nov 2021 15:21:28 +0100
Subject: [PATCH 5/8] extract where clause into seperate if + switch
---
.../HTTPRequestStateMachine.swift | 75 +++++++++++--------
1 file changed, 43 insertions(+), 32 deletions(-)
diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
index a3a9c38e0..7619b9214 100644
--- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
+++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
@@ -190,6 +190,49 @@ struct HTTPRequestStateMachine {
}
mutating func errorHappened(_ error: Error) -> Action {
+ if let error = error as? NIOSSLError, error == .uncleanShutdown {
+ switch self.state {
+ case .initialized:
+ preconditionFailure("After the state machine has been initialized, start must be called immediately. Thus this state is unreachable")
+
+ case .modifying:
+ preconditionFailure("Invalid state: \(self.state)")
+
+ case .running(.streaming, .waitingForHead),
+ .running(.endSent, .waitingForHead):
+ // if we received a NIOSSL.uncleanShutdown before we got an answer we should handle
+ // this like a normal connection close. We will receive a call to channelInactive after
+ // this error.
+ return .wait
+
+ case .running(.streaming, .receivingBody(let responseHead, _)),
+ .running(.endSent, .receivingBody(let responseHead, _)):
+ // This code is only reachable for request and responses, which we expect to have a body.
+ // We depend on logic from the HTTPResponseDecoder here. The decoder will emit an
+ // HTTPResponsePart.end right after the HTTPResponsePart.head, for every request with a
+ // CONNECT or HEAD method and every response with a 1xx, 204 or 304 response status.
+ //
+ // For this reason we only need to check the "content-length" or "transfer-encoding"
+ // headers here to determine if we are potentially in an EOF terminated response.
+
+ if responseHead.headers.contains(name: "content-length") || responseHead.headers.contains(name: "transfer-encoding") {
+ // If we have already received the response head, the parser will ensure that we
+ // receive a complete response, if the content-length or transfer-encoding header
+ // was set. In this case we can ignore the NIOSSLError.uncleanShutdown. We will see
+ // a HTTPParserError very soon.
+ return .wait
+ }
+
+ // If the response is EOF terminated, we need to rely on a clean tls shutdown to be sure
+ // we have received all necessary bytes. For this reason we forward the uncleanShutdown
+ // error to the user.
+ self.state = .failed(error)
+ return .failRequest(error, .close)
+
+ case .waitForChannelToBecomeWritable, .running, .finished, .failed:
+ break
+ }
+ }
switch self.state {
case .initialized:
preconditionFailure("After the state machine has been initialized, start must be called immediately. Thus this state is unreachable")
@@ -197,38 +240,6 @@ struct HTTPRequestStateMachine {
// the request failed, before it was sent onto the wire.
self.state = .failed(error)
return .failRequest(error, .none)
-
- case .running(.streaming, .waitingForHead) where error as? NIOSSLError == .uncleanShutdown,
- .running(.endSent, .waitingForHead) where error as? NIOSSLError == .uncleanShutdown:
- // if we received a NIOSSL.uncleanShutdown before we got an answer we should handle
- // this like a normal connection close. We will receive a call to channelInactive after
- // this error.
- return .wait
-
- case .running(.streaming, .receivingBody(let responseHead, _)) where error as? NIOSSLError == .uncleanShutdown,
- .running(.endSent, .receivingBody(let responseHead, _)) where error as? NIOSSLError == .uncleanShutdown:
- // This code is only reachable for request and responses, which we expect to have a body.
- // We depend on logic from the HTTPResponseDecoder here. The decoder will emit an
- // HTTPResponsePart.end right after the HTTPResponsePart.head, for every request with a
- // CONNECT or HEAD method and every response with a 1xx, 204 or 304 response status.
- //
- // For this reason we only need to check the "content-length" or "transfer-encoding"
- // headers here to determine if we are potentially in an EOF terminated response.
-
- if responseHead.headers.contains(name: "content-length") || responseHead.headers.contains(name: "transfer-encoding") {
- // If we have already received the response head, the parser will ensure that we
- // receive a complete response, if the content-length or transfer-encoding header
- // was set. In this case we can ignore the NIOSSLError.uncleanShutdown. We will see
- // a HTTPParserError very soon.
- return .wait
- }
-
- // If the response is EOF terminated, we need to rely on a clean tls shutdown to be sure
- // we have received all necessary bytes. For this reason we forward the uncleanShutdown
- // error to the user.
- self.state = .failed(error)
- return .failRequest(error, .close)
-
case .running:
self.state = .failed(error)
return .failRequest(error, .close)
From 2f25c2f00ee48da9e03a8b2872311d534410a3a3 Mon Sep 17 00:00:00 2001
From: David Nadoba <d_nadoba@apple.com>
Date: Wed, 24 Nov 2021 15:27:52 +0100
Subject: [PATCH 6/8] move `.uncleanShutdown` handling into seperate function
---
.../HTTPRequestStateMachine.swift | 78 +++++++++----------
1 file changed, 39 insertions(+), 39 deletions(-)
diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
index 7619b9214..4e23d431e 100644
--- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
+++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
@@ -189,49 +189,49 @@ struct HTTPRequestStateMachine {
}
}
- mutating func errorHappened(_ error: Error) -> Action {
- if let error = error as? NIOSSLError, error == .uncleanShutdown {
- switch self.state {
- case .initialized:
- preconditionFailure("After the state machine has been initialized, start must be called immediately. Thus this state is unreachable")
-
- case .modifying:
- preconditionFailure("Invalid state: \(self.state)")
-
- case .running(.streaming, .waitingForHead),
- .running(.endSent, .waitingForHead):
- // if we received a NIOSSL.uncleanShutdown before we got an answer we should handle
- // this like a normal connection close. We will receive a call to channelInactive after
- // this error.
+ mutating func handleNIOSSLUncleanShutdownError() -> Action? {
+ switch self.state {
+ case .running(.streaming, .waitingForHead),
+ .running(.endSent, .waitingForHead):
+ // if we received a NIOSSL.uncleanShutdown before we got an answer we should handle
+ // this like a normal connection close. We will receive a call to channelInactive after
+ // this error.
+ return .wait
+
+ case .running(.streaming, .receivingBody(let responseHead, _)),
+ .running(.endSent, .receivingBody(let responseHead, _)):
+ // This code is only reachable for request and responses, which we expect to have a body.
+ // We depend on logic from the HTTPResponseDecoder here. The decoder will emit an
+ // HTTPResponsePart.end right after the HTTPResponsePart.head, for every request with a
+ // CONNECT or HEAD method and every response with a 1xx, 204 or 304 response status.
+ //
+ // For this reason we only need to check the "content-length" or "transfer-encoding"
+ // headers here to determine if we are potentially in an EOF terminated response.
+
+ if responseHead.headers.contains(name: "content-length") || responseHead.headers.contains(name: "transfer-encoding") {
+ // If we have already received the response head, the parser will ensure that we
+ // receive a complete response, if the content-length or transfer-encoding header
+ // was set. In this case we can ignore the NIOSSLError.uncleanShutdown. We will see
+ // a HTTPParserError very soon.
return .wait
+ }
- case .running(.streaming, .receivingBody(let responseHead, _)),
- .running(.endSent, .receivingBody(let responseHead, _)):
- // This code is only reachable for request and responses, which we expect to have a body.
- // We depend on logic from the HTTPResponseDecoder here. The decoder will emit an
- // HTTPResponsePart.end right after the HTTPResponsePart.head, for every request with a
- // CONNECT or HEAD method and every response with a 1xx, 204 or 304 response status.
- //
- // For this reason we only need to check the "content-length" or "transfer-encoding"
- // headers here to determine if we are potentially in an EOF terminated response.
-
- if responseHead.headers.contains(name: "content-length") || responseHead.headers.contains(name: "transfer-encoding") {
- // If we have already received the response head, the parser will ensure that we
- // receive a complete response, if the content-length or transfer-encoding header
- // was set. In this case we can ignore the NIOSSLError.uncleanShutdown. We will see
- // a HTTPParserError very soon.
- return .wait
- }
+ // If the response is EOF terminated, we need to rely on a clean tls shutdown to be sure
+ // we have received all necessary bytes. For this reason we forward the uncleanShutdown
+ // error to the user.
+ self.state = .failed(error)
+ return .failRequest(error, .close)
- // If the response is EOF terminated, we need to rely on a clean tls shutdown to be sure
- // we have received all necessary bytes. For this reason we forward the uncleanShutdown
- // error to the user.
- self.state = .failed(error)
- return .failRequest(error, .close)
+ case .waitForChannelToBecomeWritable, .running, .finished, .failed, .initialized, .modifying:
+ return nil
+ }
+ }
- case .waitForChannelToBecomeWritable, .running, .finished, .failed:
- break
- }
+ mutating func errorHappened(_ error: Error) -> Action {
+ if let error = error as? NIOSSLError,
+ error == .uncleanShutdown,
+ let action = self.handleNIOSSLUncleanShutdownError() {
+ return action
}
switch self.state {
case .initialized:
From 87ab3ca480cb8dec263d11d87bdb828cad981c4a Mon Sep 17 00:00:00 2001
From: David Nadoba <d_nadoba@apple.com>
Date: Wed, 24 Nov 2021 15:30:26 +0100
Subject: [PATCH 7/8] make private and move`handleNIOSSLUncleanShutdownError`
below `errorHappened`
---
.../HTTPRequestStateMachine.swift | 54 +++++++++----------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
index 4e23d431e..ad27feac5 100644
--- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
+++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
@@ -189,7 +189,33 @@ struct HTTPRequestStateMachine {
}
}
- mutating func handleNIOSSLUncleanShutdownError() -> Action? {
+ mutating func errorHappened(_ error: Error) -> Action {
+ if let error = error as? NIOSSLError,
+ error == .uncleanShutdown,
+ let action = self.handleNIOSSLUncleanShutdownError() {
+ return action
+ }
+ switch self.state {
+ case .initialized:
+ preconditionFailure("After the state machine has been initialized, start must be called immediately. Thus this state is unreachable")
+ case .waitForChannelToBecomeWritable:
+ // the request failed, before it was sent onto the wire.
+ self.state = .failed(error)
+ return .failRequest(error, .none)
+ case .running:
+ self.state = .failed(error)
+ return .failRequest(error, .close)
+
+ case .finished, .failed:
+ // ignore error
+ return .wait
+
+ case .modifying:
+ preconditionFailure("Invalid state: \(self.state)")
+ }
+ }
+
+ private mutating func handleNIOSSLUncleanShutdownError() -> Action? {
switch self.state {
case .running(.streaming, .waitingForHead),
.running(.endSent, .waitingForHead):
@@ -227,32 +253,6 @@ struct HTTPRequestStateMachine {
}
}
- mutating func errorHappened(_ error: Error) -> Action {
- if let error = error as? NIOSSLError,
- error == .uncleanShutdown,
- let action = self.handleNIOSSLUncleanShutdownError() {
- return action
- }
- switch self.state {
- case .initialized:
- preconditionFailure("After the state machine has been initialized, start must be called immediately. Thus this state is unreachable")
- case .waitForChannelToBecomeWritable:
- // the request failed, before it was sent onto the wire.
- self.state = .failed(error)
- return .failRequest(error, .none)
- case .running:
- self.state = .failed(error)
- return .failRequest(error, .close)
-
- case .finished, .failed:
- // ignore error
- return .wait
-
- case .modifying:
- preconditionFailure("Invalid state: \(self.state)")
- }
- }
-
mutating func requestStreamPartReceived(_ part: IOData) -> Action {
switch self.state {
case .initialized,
From d45d7970e543bba7dbf77eb7bf890de0cf03f404 Mon Sep 17 00:00:00 2001
From: David Nadoba <d_nadoba@apple.com>
Date: Wed, 24 Nov 2021 15:32:50 +0100
Subject: [PATCH 8/8] fix compilation
---
.../ConnectionPool/HTTPRequestStateMachine.swift | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
index ad27feac5..380f7386e 100644
--- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
+++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPRequestStateMachine.swift
@@ -245,8 +245,8 @@ struct HTTPRequestStateMachine {
// If the response is EOF terminated, we need to rely on a clean tls shutdown to be sure
// we have received all necessary bytes. For this reason we forward the uncleanShutdown
// error to the user.
- self.state = .failed(error)
- return .failRequest(error, .close)
+ self.state = .failed(NIOSSLError.uncleanShutdown)
+ return .failRequest(NIOSSLError.uncleanShutdown, .close)
case .waitForChannelToBecomeWritable, .running, .finished, .failed, .initialized, .modifying:
return nil