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

Provide a function to write errors to a response from Express/Fastify/Next.js middleware #672

Open
timostamm opened this issue Jun 9, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@timostamm
Copy link
Member

Is your feature request related to a problem? Please describe.
When combining middleware with Connect routes, it is often necessary to return an error to the wire (for example a middleware to authenticate requests).

This is relatively straight-forward for Connect unary. Here is an example for Express with Connect unary:

import express from "express";
import {ConnectError} from "@bufbuild/connect";
import {codeToHttpStatus, errorToJson} from "@bufbuild/connect/protocol-connect";

// write a Connect error to an Express response
export function writeError(res: express.Response, error: ConnectError): void {
  res
    .status(codeToHttpStatus(error.code))
    .json(errorToJson(error, {}))
    .end();
}

With streaming, or the other two protocols, it is less straight-forward.

Describe the solution you'd like
To solve this, we can create a function in @bufbuild/connect/protocol that takes a request content-type and a ConnectError as arguments, and returns a UniversalServerResponse with the appropriate headers and body.

Then we can export functions specific to server frameworks, that make use of the function we created, but also write the universal response to the server frameworks response object.

Additional context
This feature has some overlap with #527, but would still be useful in many situations as an alternative to server-side interceptors.

@timostamm timostamm added the enhancement New feature or request label Jun 9, 2023
@CaptainWang98
Copy link

Hi, @timostamm . I found connect-es is so cool, and I'm willing to help deal with this issue. Is there a CONTRIBUTE Guide or something that introduce me about the code style or what test should be passed? So that I can provide what the community need. Thanks~

@timostamm
Copy link
Member Author

Hey @CaptainWang98, here is our CONTRIBUTING.md. It has instructions for everything you need to get started 🙂

If you follow the solution described above, you can add tests and run them with make testconnectpackage. Please note that the outline for the solution is a good plan, but there may be unforeseen issues and it might take a couple of rounds to get it right. But any good progress would be helpful and be a welcome contribution.

@CaptainWang98
Copy link

Thank you for your patience @timostamm , I'm working on this issue now. Since I have less experience in open source community, it may takes me weeks. If I have some questions, I will contact you here~

@CaptainWang98
Copy link

CaptainWang98 commented Aug 23, 2023

I have had a glance to source code, and I think I should first give out a interface like this:
export interface connectErrorToUniversalResponse { (contentType: string | null, error: ConnectError): UniversalServerResponse; }
And then, maybe implement a base version of the function connectErrorToUniversalResponse, finally implement some variants for connect-express / connect-fastify and so on.
Is this OK? I would be appreciate If get some advice from you @timostamm 😊

@timostamm
Copy link
Member Author

@CaptainWang98, defining the function first is a good idea. We are using type to declare functions types in this repository. Following this convention, the definition would be:

type ConnectErrorToUniversalResponseFn = 
  (contentType: string | null, error: ConnectError) => UniversalServerResponse;

One suggestion: To make it obvious that the first argument is the request content type, we can name it requestContentType.

Other than that, this is a great starting point. You could try to implement the function for the Connect protocol.

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

No branches or pull requests

2 participants