Skip to content

Commit 78db67e

Browse files
dnadobadnadoba
authored
Tolerate new request after connection error happened (#688)
* Tolerate new request after connection error happened * fix tests
1 parent d62c475 commit 78db67e

File tree

2 files changed

+41
-25
lines changed

2 files changed

+41
-25
lines changed

‎Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ConnectionStateMachine.swift‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,14 @@ struct HTTP1ConnectionStateMachine {
106106
return .wait
107107

108108
case .modifying:
109-
preconditionFailure("Invalid state: \(self.state)")
109+
fatalError("Invalid state: \(self.state)")
110110
}
111111
}
112112

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

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

132132
case .modifying:
133-
preconditionFailure("Invalid state: \(self.state)")
133+
fatalError("Invalid state: \(self.state)")
134134
}
135135
}
136136

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

157157
case .modifying:
158-
preconditionFailure("Invalid state: \(self.state)")
158+
fatalError("Invalid state: \(self.state)")
159159
}
160160
}
161161

@@ -173,7 +173,7 @@ struct HTTP1ConnectionStateMachine {
173173
}
174174

175175
case .modifying:
176-
preconditionFailure("Invalid state: \(self.state)")
176+
fatalError("Invalid state: \(self.state)")
177177
}
178178
}
179179

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

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

210210
case .modifying:
211-
preconditionFailure("Invalid state: \(self.state)")
211+
fatalError("Invalid state: \(self.state)")
212212
}
213213
}
214214

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

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

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

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

244244
case .idle:
245245
if closeConnection {
@@ -260,7 +260,7 @@ struct HTTP1ConnectionStateMachine {
260260
return .wait
261261

262262
case .modifying:
263-
preconditionFailure("Invalid state: \(self.state)")
263+
fatalError("Invalid state: \(self.state)")
264264
}
265265
}
266266

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

286286
case .modifying:
287-
preconditionFailure("Invalid state: \(self.state)")
287+
fatalError("Invalid state: \(self.state)")
288288
}
289289
}
290290

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

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

312312
case .modifying:
313-
preconditionFailure("Invalid state: \(self.state)")
313+
fatalError("Invalid state: \(self.state)")
314314
}
315315
}
316316

@@ -327,13 +327,13 @@ struct HTTP1ConnectionStateMachine {
327327
}
328328

329329
case .modifying:
330-
preconditionFailure("Invalid state: \(self.state)")
330+
fatalError("Invalid state: \(self.state)")
331331
}
332332
}
333333

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

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

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

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

429429
let newFinalAction: HTTP1ConnectionStateMachine.Action.FinalSuccessfulStreamAction
@@ -443,9 +443,9 @@ extension HTTP1ConnectionStateMachine.State {
443443
case .failRequest(let error, let finalAction):
444444
switch self {
445445
case .initialized:
446-
preconditionFailure("Invalid state: \(self)")
446+
fatalError("Invalid state: \(self)")
447447
case .idle:
448-
preconditionFailure("How can we fail a task, if we are idle")
448+
fatalError("How can we fail a task, if we are idle")
449449
case .inRequest(_, close: let close):
450450
if case .close(let promise) = finalAction {
451451
self = .closing
@@ -465,7 +465,7 @@ extension HTTP1ConnectionStateMachine.State {
465465
return .failRequest(error, .none)
466466

467467
case .modifying:
468-
preconditionFailure("Invalid state: \(self)")
468+
fatalError("Invalid state: \(self)")
469469
}
470470

471471
case .read:
@@ -497,7 +497,7 @@ extension HTTP1ConnectionStateMachine: CustomStringConvertible {
497497
case .closed:
498498
return ".closed"
499499
case .modifying:
500-
preconditionFailure("Invalid state: \(self.state)")
500+
fatalError("Invalid state: \(self.state)")
501501
}
502502
}
503503
}

‎Tests/AsyncHTTPClientTests/HTTP1ConnectionStateMachineTests.swift‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,19 @@ class HTTP1ConnectionStateMachineTests: XCTestCase {
200200
XCTAssertEqual(state.requestCancelled(closeConnection: false), .failRequest(HTTPClientError.cancelled, .close(nil)))
201201
}
202202

203+
func testNewRequestAfterErrorHappened() {
204+
var state = HTTP1ConnectionStateMachine()
205+
XCTAssertEqual(state.channelActive(isWritable: false), .fireChannelActive)
206+
struct MyError: Error, Equatable {}
207+
XCTAssertEqual(state.errorHappened(MyError()), .fireChannelError(MyError(), closeConnection: true))
208+
let requestHead = HTTPRequestHead(version: .http1_1, method: .POST, uri: "/", headers: ["content-length": "4"])
209+
let metadata = RequestFramingMetadata(connectionClose: false, body: .fixedSize(4))
210+
let action = state.runNewRequest(head: requestHead, metadata: metadata)
211+
guard case .failRequest = action else {
212+
return XCTFail("unexpected action \(action)")
213+
}
214+
}
215+
203216
func testCancelRequestIsIgnoredWhenConnectionIsIdle() {
204217
var state = HTTP1ConnectionStateMachine()
205218
XCTAssertEqual(state.channelActive(isWritable: true), .fireChannelActive)
@@ -303,6 +316,8 @@ extension HTTP1ConnectionStateMachine.Action: Equatable {
303316

304317
case (.fireChannelInactive, .fireChannelInactive):
305318
return true
319+
case (.fireChannelError(_, let lhsCloseConnection), .fireChannelError(_, let rhsCloseConnection)):
320+
return lhsCloseConnection == rhsCloseConnection
306321

307322
case (.sendRequestHead(let lhsHead, let lhsStartBody), .sendRequestHead(let rhsHead, let rhsStartBody)):
308323
return lhsHead == rhsHead && lhsStartBody == rhsStartBody
@@ -377,6 +392,7 @@ extension HTTP1ConnectionStateMachine.Action.FinalFailedStreamAction: Equatable
377392
return lhsPromise?.futureResult == rhsPromise?.futureResult
378393
case (.none, .none):
379394
return true
395+
380396
default:
381397
return false
382398
}

0 commit comments

Comments
 (0)