Skip to content

Commit

Permalink
Patch configuration and log actual port on startup (#3160)
Browse files Browse the repository at this point in the history
* Log actual port on startup

Before this change, the application

```swift
let app = Application(.testing)
defer { app.shutdown() }
try app.server.start(hostname: nil, port: 0)
defer { app.server.shutdown() }
```

would log the following message *before* starting the server:

```
[Vapor] Server starting on http://127.0.0.1:0
```

After this change it instead logs a message like the following *after* starting the server:

```
[Vapor] Server started on [IPv4]127.0.0.1/127.0.0.1:57935
```

* Avoid changing log format

* Log both before and after starting

* Make 'addressDescription' a computed property

Just invoking the property twice seemed cleaner to me compared to duplicating the code block. Fixed a little indentation issue in the process

I don't know if it's ever relevant, but config is now patched on all address types, including unix socket.

* Remove obsolete variable

* Make 'addressDescription' internal

* Add test

* Fix broken test

---------

Co-authored-by: Tim Condon <0xTim@users.noreply.github.com>
  • Loading branch information
bisgardo and 0xTim committed Apr 26, 2024
1 parent 489acd9 commit e5cb0ab
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 31 deletions.
73 changes: 44 additions & 29 deletions Sources/Vapor/HTTP/Server/HTTPServer.swift
Expand Up @@ -48,23 +48,34 @@ public final class HTTPServer: Server, Sendable {

/// Port the server will bind to.
public var port: Int {
get {
switch address {
case .hostname(_, let port):
return port ?? Self.defaultPort
default:
return Self.defaultPort
}
}
set {
switch address {
case .hostname(let hostname, _):
address = .hostname(hostname, port: newValue)
default:
address = .hostname(nil, port: newValue)
}
}
}
get {
switch address {
case .hostname(_, let port):
return port ?? Self.defaultPort
default:
return Self.defaultPort
}
}
set {
switch address {
case .hostname(let hostname, _):
address = .hostname(hostname, port: newValue)
default:
address = .hostname(nil, port: newValue)
}
}
}

/// A human-readable description of the configured address. Used in log messages when starting server.
var addressDescription: String {
let scheme = tlsConfiguration == nil ? "http" : "https"
switch address {
case .hostname(let hostname, let port):
return "\(scheme)://\(hostname ?? Self.defaultHostname):\(port ?? Self.defaultPort)"
case .unixDomainSocket(let socketPath):
return "\(scheme)+unix: \(socketPath)"
}
}

/// Listen backlog.
public var backlog: Int
Expand Down Expand Up @@ -307,19 +318,10 @@ public final class HTTPServer: Server, Sendable {
/// Override the socket path.
configuration.address = address!
}

/// Print starting message.
let scheme = configuration.tlsConfiguration == nil ? "http" : "https"
let addressDescription: String
switch configuration.address {
case .hostname(let hostname, let port):
addressDescription = "\(scheme)://\(hostname ?? configuration.hostname):\(port ?? configuration.port)"
case .unixDomainSocket(let socketPath):
addressDescription = "\(scheme)+unix: \(socketPath)"
}

self.configuration.logger.notice("Server starting on \(addressDescription)")

/// Log starting message for debugging before attempting to start the server.
configuration.logger.debug("Server starting on \(configuration.addressDescription)")

/// Start the actual `HTTPServer`.
try self.connection.withLockedValue {
$0 = try HTTPServerConnection.start(
Expand All @@ -331,6 +333,19 @@ public final class HTTPServer: Server, Sendable {
).wait()
}

/// Overwrite configuration with actual address, if applicable.
/// They may differ from the provided configuation if port 0 was provided, for example.
if let localAddress = self.localAddress {
if let hostname = localAddress.hostname, let port = localAddress.port {
configuration.address = .hostname(hostname, port: port)
} else if let pathname = localAddress.pathname {
configuration.address = .unixDomainSocket(path: pathname)
}
}

/// Log started message with the actual configuration.
configuration.logger.notice("Server started on \(configuration.addressDescription)")

self.configuration = configuration
self.didStart.withLockedValue { $0 = true }
}
Expand Down
4 changes: 2 additions & 2 deletions Tests/VaporTests/ApplicationTests.swift
Expand Up @@ -179,7 +179,7 @@ final class ApplicationTests: XCTestCase {

XCTAssertNotNil(app.http.server.shared.localAddress)
XCTAssertEqual("0.0.0.0", app.http.server.configuration.hostname)
XCTAssertEqual(0, app.http.server.configuration.port)
XCTAssertEqual(app.http.server.shared.localAddress?.port, app.http.server.configuration.port)

guard let localAddress = app.http.server.shared.localAddress,
localAddress.ipAddress != nil,
Expand All @@ -190,7 +190,7 @@ final class ApplicationTests: XCTestCase {
let response = try app.client.get("http://localhost:\(port)/hello").wait()
let returnedConfig = try response.content.decode(AddressConfig.self)
XCTAssertEqual(returnedConfig.hostname, "0.0.0.0")
XCTAssertEqual(returnedConfig.port, 0)
XCTAssertEqual(returnedConfig.port, port)
}

func testConfigurationAddressDetailsReflectedWhenProvidedThroughServeCommand() throws {
Expand Down
10 changes: 10 additions & 0 deletions Tests/VaporTests/ServerTests.swift
Expand Up @@ -1409,6 +1409,16 @@ final class ServerTests: XCTestCase {
}
}

func testConfigurationHasActualPortAfterStart() throws {
let app = Application(.testing)
app.http.server.configuration.port = 0
defer { app.shutdown() }
try app.start()

XCTAssertNotEqual(app.http.server.configuration.port, 0)
XCTAssertEqual(app.http.server.configuration.port, app.http.server.shared.localAddress?.port)
}

override class func setUp() {
XCTAssertTrue(isLoggingConfigured)
}
Expand Down

0 comments on commit e5cb0ab

Please sign in to comment.