Skip to content

Commit

Permalink
Tolerate new request after connection error happened (#688)
Browse files Browse the repository at this point in the history 
* Tolerate new request after connection error happened

* fix tests
  • Loading branch information
@dnadoba
dnadoba authored May 17, 2023
1 parent d62c475 commit 78db67e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 25 deletions.
50 changes: 25 additions & 25 deletions Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ConnectionStateMachine.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,14 @@ struct HTTP1ConnectionStateMachine {
return .wait

case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}

mutating func channelInactive() -> Action {
switch self.state {
case .initialized:
preconditionFailure("A channel that isn't active, must not become inactive")
fatalError("A channel that isn't active, must not become inactive")

case .inRequest(var requestStateMachine, close: _):
return self.avoidingStateMachineCoW { state -> Action in
Expand All @@ -130,7 +130,7 @@ struct HTTP1ConnectionStateMachine {
return .wait

case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}

Expand All @@ -155,7 +155,7 @@ struct HTTP1ConnectionStateMachine {
return .fireChannelError(error, closeConnection: false)

case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}

Expand All @@ -173,7 +173,7 @@ struct HTTP1ConnectionStateMachine {
}

case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}

Expand All @@ -182,15 +182,15 @@ struct HTTP1ConnectionStateMachine {
metadata: RequestFramingMetadata
) -> Action {
switch self.state {
case .initialized, .closing, .inRequest:
case .initialized, .inRequest:
// These states are unreachable as the connection pool state machine has put the
// connection into these states. In other words the connection pool state machine must
// be aware about these states before the connection itself. For this reason the
// connection pool state machine must not send a new request to the connection, if the
// connection is `.initialized`, `.closing` or `.inRequest`
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")

case .closed:
case .closing, .closed:
// The remote may have closed the connection and the connection pool state machine
// was not updated yet because of a race condition. New request vs. marking connection
// as closed.
Expand All @@ -208,13 +208,13 @@ struct HTTP1ConnectionStateMachine {
return self.state.modify(with: action)

case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}

mutating func requestStreamPartReceived(_ part: IOData, promise: EventLoopPromise<Void>?) -> Action {
guard case .inRequest(var requestStateMachine, let close) = self.state else {
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}

return self.avoidingStateMachineCoW { state -> Action in
Expand All @@ -226,7 +226,7 @@ struct HTTP1ConnectionStateMachine {

mutating func requestStreamFinished(promise: EventLoopPromise<Void>?) -> Action {
guard case .inRequest(var requestStateMachine, let close) = self.state else {
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}

return self.avoidingStateMachineCoW { state -> Action in
Expand All @@ -239,7 +239,7 @@ struct HTTP1ConnectionStateMachine {
mutating func requestCancelled(closeConnection: Bool) -> Action {
switch self.state {
case .initialized:
preconditionFailure("This event must only happen, if the connection is leased. During startup this is impossible. Invalid state: \(self.state)")
fatalError("This event must only happen, if the connection is leased. During startup this is impossible. Invalid state: \(self.state)")

case .idle:
if closeConnection {
Expand All @@ -260,7 +260,7 @@ struct HTTP1ConnectionStateMachine {
return .wait

case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}

Expand All @@ -269,7 +269,7 @@ struct HTTP1ConnectionStateMachine {
mutating func read() -> Action {
switch self.state {
case .initialized:
preconditionFailure("Why should we read something, if we are not connected yet")
fatalError("Why should we read something, if we are not connected yet")
case .idle:
return .read
case .inRequest(var requestStateMachine, let close):
Expand All @@ -284,14 +284,14 @@ struct HTTP1ConnectionStateMachine {
return .read

case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}

mutating func channelRead(_ part: HTTPClientResponsePart) -> Action {
switch self.state {
case .initialized, .idle:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")

case .inRequest(var requestStateMachine, var close):
return self.avoidingStateMachineCoW { state -> Action in
Expand All @@ -310,7 +310,7 @@ struct HTTP1ConnectionStateMachine {
return .wait

case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}

Expand All @@ -327,13 +327,13 @@ struct HTTP1ConnectionStateMachine {
}

case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}

mutating func demandMoreResponseBodyParts() -> Action {
guard case .inRequest(var requestStateMachine, let close) = self.state else {
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}

return self.avoidingStateMachineCoW { state -> Action in
Expand All @@ -345,7 +345,7 @@ struct HTTP1ConnectionStateMachine {

mutating func idleReadTimeoutTriggered() -> Action {
guard case .inRequest(var requestStateMachine, let close) = self.state else {
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}

return self.avoidingStateMachineCoW { state -> Action in
Expand Down Expand Up @@ -423,7 +423,7 @@ extension HTTP1ConnectionStateMachine.State {
return .forwardResponseBodyParts(parts)
case .succeedRequest(let finalAction, let finalParts):
guard case .inRequest(_, close: let close) = self else {
preconditionFailure("Invalid state: \(self)")
fatalError("Invalid state: \(self)")
}

let newFinalAction: HTTP1ConnectionStateMachine.Action.FinalSuccessfulStreamAction
Expand All @@ -443,9 +443,9 @@ extension HTTP1ConnectionStateMachine.State {
case .failRequest(let error, let finalAction):
switch self {
case .initialized:
preconditionFailure("Invalid state: \(self)")
fatalError("Invalid state: \(self)")
case .idle:
preconditionFailure("How can we fail a task, if we are idle")
fatalError("How can we fail a task, if we are idle")
case .inRequest(_, close: let close):
if case .close(let promise) = finalAction {
self = .closing
Expand All @@ -465,7 +465,7 @@ extension HTTP1ConnectionStateMachine.State {
return .failRequest(error, .none)

case .modifying:
preconditionFailure("Invalid state: \(self)")
fatalError("Invalid state: \(self)")
}

case .read:
Expand Down Expand Up @@ -497,7 +497,7 @@ extension HTTP1ConnectionStateMachine: CustomStringConvertible {
case .closed:
return ".closed"
case .modifying:
preconditionFailure("Invalid state: \(self.state)")
fatalError("Invalid state: \(self.state)")
}
}
}
16 changes: 16 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,19 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
XCTAssertEqual(state.requestCancelled(closeConnection: false), .failRequest(HTTPClientError.cancelled, .close(nil)))
}

func testNewRequestAfterErrorHappened() {
var state = HTTP1ConnectionStateMachine()
XCTAssertEqual(state.channelActive(isWritable: false), .fireChannelActive)
struct MyError: Error, Equatable {}
XCTAssertEqual(state.errorHappened(MyError()), .fireChannelError(MyError(), closeConnection: true))
let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: ["content-length": "4"])
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
let action = state.runNewRequest(head: requestHead, metadata: metadata)
guard case .failRequest = action else {
return XCTFail("unexpected action \(action)")
}
}

func testCancelRequestIsIgnoredWhenConnectionIsIdle() {
var state = HTTP1ConnectionStateMachine()
XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
Expand Down Expand Up @@ -303,6 +316,8 @@ extension HTTP1ConnectionStateMachine.Action: Equatable {

case (.fireChannelInactive, .fireChannelInactive):
return true
case (.fireChannelError(_, let lhsCloseConnection), .fireChannelError(_, let rhsCloseConnection)):
return lhsCloseConnection == rhsCloseConnection

case (.sendRequestHead(let lhsHead, let lhsStartBody), .sendRequestHead(let rhsHead, let rhsStartBody)):
return lhsHead == rhsHead && lhsStartBody == rhsStartBody
Expand Down Expand Up @@ -377,6 +392,7 @@ extension HTTP1ConnectionStateMachine.Action.FinalFailedStreamAction: Equatable
return lhsPromise?.futureResult == rhsPromise?.futureResult
case (.none, .none):
return true

default:
return false
}
Expand Down

0 comments on commit 78db67e

Please sign in to comment.