Transition HBResponder to be async/await, removing HBAsyncResponder in the process #256
Conversation
Codecov ReportAttention:
... and 3 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
| return endpointPath | ||
| }() | ||
|
|
||
| return context.withSpan(operationName, serviceContext: serviceContext, ofKind: .server) { context, span in |
There was a problem hiding this comment.
You can't use context.withSpan here. You need to use ServiceContext.withSpan. The whole of ServiceContext.swift needs to be deleted. It is there to preserve the service context between async and non-async contexts.
There was a problem hiding this comment.
Isn't this applicable only after we land the HBMiddleware PR?
| var serviceContext = context.serviceContext | ||
| serviceContext.testID = "testMiddleware" | ||
|
|
||
| return try await context.withSpan("TestSpan", serviceContext: serviceContext, ofKind: .server) { context, _ in |
There was a problem hiding this comment.
Use InstrumentationSystem.tracer.withSpan
| @@ -323,7 +323,7 @@ final class TracingTests: XCTestCase { | |||
| app.router.get("/") { _, context -> HTTPResponseStatus in | |||
| var serviceContext = context.serviceContext | |||
| serviceContext.testID = "test" | |||
| return context.withSpan("TestSpan", serviceContext: serviceContext, ofKind: .client) { _, span in | |||
| return try await context.withSpan("TestSpan", serviceContext: serviceContext, ofKind: .client) { _, span in | |||
There was a problem hiding this comment.
Use InstrumentationSystem.tracer.withSpan
| @@ -449,7 +452,7 @@ extension TracingTests { | |||
| app.middleware.add(AsyncSpanMiddleware()) | |||
| app.router.get("/") { _, context -> HTTPResponseStatus in | |||
| try await Task.sleep(nanoseconds: 1000) | |||
| return context.withSpan("testing", ofKind: .server) { _, _ in | |||
| return try await context.withSpan("testing", ofKind: .server) { _, _ in | |||
There was a problem hiding this comment.
Use InstrumentationSystem.tracer.withSpan
| @@ -49,7 +49,7 @@ struct User: Decodable { | |||
| app.router.post("user") { request -> EventLoopFuture<HTTPResponseStatus> in | |||
There was a problem hiding this comment.
Nice documentation update. Not many people consider that
| // respond to request | ||
| self.responder.respond(to: request, context: context).whenComplete { result in |
There was a problem hiding this comment.
We can leave this for another PR
…n the process This PR provides a compatibility helper for APIs still working with ELFs, primarily the Middleware
Joannis
force-pushed
the
jo/hb-responder-async
branch
from
79e8382 to
0994b39
Compare
This PR provides a compatibility helper for APIs still working with ELFs, primarily the Middleware