make sure HTTPClient is shutdown (#98)

weissi · web-flow · commit 26afbc14bb87 · 2019-09-05T15:33:13.000+01:00
Motivation:

Right now, HTTPClient only asserts that it's shut down if it was started with its own EventLoopGroup.
That however is weird because it's lifecycle model depends on the parameters you pass to `init`.

Modifications:

Always validate the lifecycle (in debug mode).

Result:

API makes more sense.
diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift
@@ -66,12 +66,7 @@ public class HTTPClient {
     }
 
     deinit {
-        switch self.eventLoopGroupProvider {
-        case .shared:
-            return
-        case .createNew:
-            assert(self.isShutdown.load(), "Client not stopped before the deinit.")
-        }
+        assert(self.isShutdown.load(), "Client not shut down before the deinit. Please call client.syncShutdown() when no longer needed.")
     }
 
     /// Shuts down the client and `EventLoopGroup` if it was created by the client.
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift
@@ -77,7 +77,7 @@ class HTTPClientInternalTests: XCTestCase {
         let httpBin = HttpBin()
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -107,7 +107,7 @@ class HTTPClientInternalTests: XCTestCase {
         let httpBin = HttpBin()
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -168,7 +168,7 @@ class HTTPClientInternalTests: XCTestCase {
         let httpBin = HttpBin(channelPromise: promise)
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift
@@ -54,7 +54,7 @@ class HTTPClientTests: XCTestCase {
         let httpBin = HttpBin()
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -67,7 +67,8 @@ class HTTPClientTests: XCTestCase {
         let elg = MultiThreadedEventLoopGroup(numberOfThreads: 8)
         let httpClient = HTTPClient(eventLoopGroupProvider: .shared(elg))
         defer {
-            try! elg.syncShutdownGracefully()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
+            XCTAssertNoThrow(try elg.syncShutdownGracefully())
             httpBin.shutdown()
         }
 
@@ -85,7 +86,7 @@ class HTTPClientTests: XCTestCase {
         let httpBin = HttpBin()
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -102,7 +103,7 @@ class HTTPClientTests: XCTestCase {
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
                                     configuration: HTTPClient.Configuration(certificateVerification: .none))
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -115,7 +116,7 @@ class HTTPClientTests: XCTestCase {
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
                                     configuration: HTTPClient.Configuration(certificateVerification: .none))
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -136,7 +137,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none, followRedirects: true))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
             httpsBin.shutdown()
         }
@@ -154,7 +155,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none, followRedirects: true))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -176,7 +177,7 @@ class HTTPClientTests: XCTestCase {
         let httpBin = HttpBin()
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -187,7 +188,7 @@ class HTTPClientTests: XCTestCase {
     func testMultipleContentLengthHeaders() throws {
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
         }
         let httpBin = HttpBin()
         defer {
@@ -208,7 +209,7 @@ class HTTPClientTests: XCTestCase {
         let httpBin = HttpBin()
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -226,7 +227,7 @@ class HTTPClientTests: XCTestCase {
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -242,7 +243,7 @@ class HTTPClientTests: XCTestCase {
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: HTTPClient.Configuration(timeout: HTTPClient.Configuration.Timeout(read: .milliseconds(150))))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -258,7 +259,7 @@ class HTTPClientTests: XCTestCase {
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -274,7 +275,7 @@ class HTTPClientTests: XCTestCase {
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -300,7 +301,7 @@ class HTTPClientTests: XCTestCase {
             configuration: .init(proxy: .server(host: "localhost", port: httpBin.port))
         )
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
         let res = try httpClient.get(url: "http://test/ok").wait()
@@ -317,7 +318,7 @@ class HTTPClientTests: XCTestCase {
             )
         )
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
         let res = try httpClient.get(url: "https://test/ok").wait()
@@ -328,7 +329,7 @@ class HTTPClientTests: XCTestCase {
         let httpBin = HttpBin()
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -354,7 +355,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -371,7 +372,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none, ignoreUncleanSSLShutdown: true))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -389,7 +390,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -407,7 +408,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -423,7 +424,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -440,7 +441,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none, ignoreUncleanSSLShutdown: true))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -457,7 +458,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -474,7 +475,7 @@ class HTTPClientTests: XCTestCase {
                                     configuration: HTTPClient.Configuration(certificateVerification: .none, ignoreUncleanSSLShutdown: true))
 
         defer {
-            try! httpClient.syncShutdown()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
             httpBin.shutdown()
         }
 
@@ -491,7 +492,8 @@ class HTTPClientTests: XCTestCase {
         let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup),
                                     configuration: HTTPClient.Configuration(followRedirects: true))
         defer {
-            try! eventLoopGroup.syncShutdownGracefully()
+            XCTAssertNoThrow(try httpClient.syncShutdown())
+            XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully())
             httpBin.shutdown()
         }
 

Original file line number	Diff line number	Diff line change
`@@ -66,12 +66,7 @@ public class HTTPClient {`
`66`	`66`	`}`
`67`	`67`
`68`	`68`	`deinit {`
`69`		`- switch self.eventLoopGroupProvider {`
`70`		`- case .shared:`
`71`		`- return`
`72`		`- case .createNew:`
`73`		`- assert(self.isShutdown.load(), "Client not stopped before the deinit.")`
`74`		`- }`
	`69`	`+ assert(self.isShutdown.load(), "Client not shut down before the deinit. Please call client.syncShutdown() when no longer needed.")`
`75`	`70`	`}`
`76`	`71`
`77`	`72`	/// Shuts down the client and `EventLoopGroup` if it was created by the client.