Possible escaping closure related Unimplemented that likely can't be fixed via withEscapedDependencies/yield

[finestructure]

This is a follow up to this discussion/solution but I thought it'd start a new one because that discussion started out about Codable but ended up being about escaping closures.

This is going to rather lenghty, apologies! I was still problem solving while writing it up and then I thought it's probably best to just leave in all the details rather than try and edit it down.

I've hit another case where I'm losing dependencies in a test and I'm not sure this is related to escaping closures, or at least not in a way where I could inject withEscapedDependencies/yield.

I have a Vapor test that correctly sets the dependency if I run a request it as follows:

            try await app.testable(method: .inMemory).test(
                .POST,
                "api/versions/\(versionId)/build-report",
                headers: .bearerApplicationJSON("secr3t"),
                body: body,
                afterResponse: { res async in
                    // validation
                    XCTAssertEqual(res.status, .noContent)
                })
        }

However, it fails with an failed - Unimplemented: 'EnvironmentClient.builderToken' if I run it as follows:

            try await app.testable(method: .running(port: outOfTheWayPort)).test(
                .POST,
                "api/versions/\(versionId)/build-report",
                headers: .bearerApplicationJSON("secr3t"),
                body: body,
                afterResponse: { res async in
                    // validation
                    XCTAssertEqual(res.status, .noContent)
                })
        }

The difference is that in the .inMemory configuration, the test essentially short-circuits the response handling without spinning up a server. In the .running case it starts a web server via app.server.start(address: .hostname(self.hostname, port: self.port)) and sends an actual HTTP request.

(We need to run the latter in this test, because the short-circuit bypasses an essential part we're trying to regression test for, which funnily enough is a regression test for a Pointfree.co project 🙂)

Best I can tell, the problem comes down to the performTest method of the two conformers Live/InMemory:

• Live (failing):

        func performTest(request: XCTHTTPRequest) async throws -> XCTHTTPResponse {
            try await app.server.start(address: .hostname(self.hostname, port: self.port))
            let client = HTTPClient(eventLoopGroup: MultiThreadedEventLoopGroup.singleton)
            
            do {
                var path = request.url.path
                path = path.hasPrefix("/") ? path : "/\(path)"
                
                let actualPort: Int
                
                if self.port == 0 {
                    guard let portAllocated = app.http.server.shared.localAddress?.port else {
                        throw Abort(.internalServerError, reason: "Failed to get port from local address")
                    }
                    actualPort = portAllocated
                } else {
                    actualPort = self.port
                }
                
                var url = "http://\(self.hostname):\(actualPort)\(path)"
                if let query = request.url.query {
                    url += "?\(query)"
                }
                var clientRequest = HTTPClientRequest(url: url)
                clientRequest.method = request.method
                clientRequest.headers = request.headers
                clientRequest.body = .bytes(request.body)
                let response = try await client.execute(clientRequest, timeout: .seconds(30))
                // Collect up to 1MB
                let responseBody = try await response.body.collect(upTo: 1024 * 1024)
                try await client.shutdown()
                await app.server.shutdown()
                return XCTHTTPResponse(
                    status: response.status,
                    headers: response.headers,
                    body: responseBody
                )
            } catch {
                try? await client.shutdown()
                await app.server.shutdown()
                throw error
            }
        }

• InMemory (succeeding):

        @discardableResult
        public func performTest(
            request: XCTHTTPRequest
        ) async throws -> XCTHTTPResponse {
            var headers = request.headers
            headers.replaceOrAdd(
                name: .contentLength,
                value: request.body.readableBytes.description
            )
            let request = Request(
                application: app,
                method: request.method,
                url: request.url,
                headers: headers,
                collectedBody: request.body.readableBytes == 0 ? nil : request.body,
                remoteAddress: nil,
                on: self.app.eventLoopGroup.next()
            )
            let res = try await self.app.responder.respond(to: request).get()
            return try await XCTHTTPResponse(
                status: res.status,
                headers: res.headers,
                body: res.body.collect(on: request.eventLoop).get() ?? ByteBufferAllocator().buffer(capacity: 0)
            )
        }

Now the dependency isn't within app itself but attached to a route on app. It's for the bearer token auth mechanism via User:

extension User {
    static var builder: Self { .init(name: "builder", identifier: "builder") }

    struct BuilderAuthenticator: AsyncBearerAuthenticator {
        func authenticate(bearer: BearerAuthorization, for request: Request) async throws {
            @Dependency(\.environment) var environment
            if let builderToken = environment.builderToken(),
               bearer.token == builderToken {
                request.auth.login(User.builder)
            }
        }
    }
}

which is attached as follows:

        app.group(User.BuilderAuthenticator(), User.guardMiddleware()) {
            $0.group(tags: []) { protected in
                protected.on(.POST, SiteURL.api(.versions(.key, .buildReport)).pathComponents,
                             body: .collect(maxSize: 100_000),
                             use: API.BuildController.buildReport)
        }

Now it's true that there is an escaping closure involved here (*), the use: in protected.on(...). However, passing the dependency through via withEscpapedDependencies and yield doesn't help. Also note, that no weaving through of dependencies is required and the dependency is definitely being updated when the test is called via the inMemory request test.

(*) The trailing closures on group aren't escaping but even so I tried weaving the dependencies through as above but to no effect.

I suspect the issue stems from starting a new server. Even though the app object should be properly instrumented with the updated dependency, inside that machinery it gets lost. In particular, I see the following inside Vapor's HTTPServerConnection:

                        }, http1ChannelConfigurator: { channel in
                            return channel.pipeline.addVaporHTTP1Handlers(
                                application: application,
                                responder: responder,
                                configuration: configuration
                            )
                        })

That's our app object being passed in here and this is the signature of NIO's Channel.configureHTTP2SecureUpgrade:

    public func configureHTTP2SecureUpgrade(h2ChannelConfigurator: @escaping NIOChannelInitializer,
                                            http1ChannelConfigurator: @escaping NIOChannelInitializer) -> EventLoopFuture<Void> {

Could that be the problem? If so, since I don't control any of the surrounding code, is there any way for me to make sure the dependency gets injected?

We're deep inside Vapor/NIO code here and I have no way to bring in a withEscapedDependencies/yield pair...

[finestructure]

Ok, I've solved my problem. I don't think it is related to @escaping closures, although I'm not sure what is causing it exactly.

The problem is with the BuilderAuthenticator I've mentioned above. Here's the problematic version:

    struct BuilderAuthenticator: AsyncBearerAuthenticator {
        func authenticate(bearer: BearerAuthorization, for request: Request) async throws {
            @Dependency(\.environment) var environment
            if let token = environment.builderToken(), bearer.token == token {
                request.auth.login(User.builder)
            }
        }
    }

The issue seems to be that for some reason, in just that one test that is set up differently, dependencies are not set when authenticate is being called.

The fix is to set the dependency when the BuilderAuthenticator is created:

    struct BuilderAuthenticator: AsyncBearerAuthenticator {
        @Dependency(\.environment) var environment

        func authenticate(bearer: BearerAuthorization, for request: Request) async throws {
            if let token = environment.builderToken(), bearer.token == token {
                request.auth.login(User.builder)
            }
        }
    }

I've also had to make sure withDependencies is called earlier in my test setup than for other tests (that also run through this auth code path). This is why I had initially discarded the above fix to the BuilderAuthenticator, because it wasn't sufficient and looked like it wasn't the correct fix.

Something about this particular test that brings up an actual server is different in ways I don't quite understand but this fixes the issues without having to modify upstream packages like Vapor or NIO.

All of this is very specific to our project but I thought that there's perhaps one bit that might help others trying to debug this sort of issue. I tried to find out where the dependencies were lost and that included forking Vapor and NIO and weaving in withEscapedDependencies in places.

None of that worked (because the problem wasn't there) but I've also since found an easier way to diagnose if the dependencies are set as expected without having to make big changes to upstream packages.

If you set breakpoints with a debugger command p DependencyValues._current you can see how the dependencies are currently set up, making it easier to see where you lose them.

(Note that you'll still need to fork upstream packages in order to add swift-dependencies as a dependency of theirs and add import Dependencies to files you want to instrument for this command to work.)

You can even drill in further, like so p DependencyValues._current[EnvironmentClient.self]._builderToken. It will show

(@Sendable () -> String?) 0x000000011e55cb94 AppTests`closure #1 @Sendable () -> Swift.Optional<Swift.String> 
in closure #1 (inout Dependencies.DependencyValues) async -> () 
in AppTests.AppTestCase.setup(Vapor.Environment) async throws -> Vapor.Application 
at AppTestCase.swift:40

showing that it's correctly set to the injected location at AppTestCase.swift:40.

If I don't override the dependencies there, it'll show

(@Sendable () -> String?) 0x0000000123a7e158 AppTests`closure #1 @Sendable () -> Swift.Optional<Swift.String> 
in variable initialization expression of App.EnvironmentClient._builderToken : @Sendable () -> Swift.Optional<Swift.String> 
at @__swiftmacro_3App17EnvironmentClientV12builderToken18DependencyEndpointfMp_.swift:1

which points to the declaration location, i.e. what will resolve to the XCTFail with Unimplemented.

Sprinkling debug breakpoints like this throughout makes it a lot easier to track this down. Hope that helps others!