Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Middleware Async #3108

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Make Middleware Async #3108

wants to merge 3 commits into from

Conversation

0xTim
Copy link
Member

@0xTim 0xTim commented Nov 30, 2023

Makes all of Vapor's internal middleware async so we can have a full async chain all the way down in anticipation of distributed tracing (and adopting NIO's new AsyncChannel stuff)

@@ -3,7 +3,7 @@ import NIOCore
import NIOHTTP1

/// Captures all errors and transforms them into an internal server error HTTP response.
public final class ErrorMiddleware: Middleware {
public final class ErrorMiddleware: AsyncMiddleware {
/// Structure of `ErrorMiddleware` default response.
internal struct ErrorResponse: Codable {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you are at it, is it possible to make ErrorResponse public?

In my project I use https://github.com/mattpolzin/VaporOpenAPI to generate the openApi.yml file from the server code and with this in turn the client facing structs - the server is the single source of truth for the models.

Right now I need to duplicate this struct and make it conform to Sampleable & OpenAPIExampleProvider for this to work, which is not ideal.

I also tried writing my own ErrorMiddleware with a public ErrorResponse struct but this failed as I couldn't use response.responseBox as its marked internal and can only be used from the vapor module.

What would be the recommended approach here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants