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

Add support for Express #479

Merged
merged 2 commits into from
Feb 22, 2023
Merged

Add support for Express #479

merged 2 commits into from
Feb 22, 2023

Conversation

timostamm
Copy link
Member

@timostamm timostamm commented Feb 21, 2023

This adds support for the web framework Express, in a similar fashion to Fastify, which was added in #474.

With the help of ConnectRouter:

// connect.ts
import { ConnectRouter } from "@bufbuild/connect";

export default function(router: ConnectRouter) {
  // implement rpc Say(SayRequest) returns (SayResponse)
  router.rpc(ElizaService, ElizaService.methods.say, async (req) => ({
    sentence: `you said: ${req.sentence}`,
  }));
}

... plugging services into an Express server becomes really straight-forward:

// server.ts
import http from "http";
import express from "express";
+ import routes from "connect";
+ import { expressConnectMiddleware } from "@bufbuild/connect-express";

const app = express();

+ app.use(expressConnectMiddleware({ 
+  routes 
+ }));

http.createServer(app).listen(8080);

Other changes:

Comment on lines +63 to +65
if (opts.acceptCompression === undefined) {
opts.acceptCompression = [compressionGzip, compressionBrotli];
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Defaults for acceptCompression moved a couple of lines up. Otherwise, createConnectRouter overrides with an empty array as a default, and these defaults never apply.

Comment on lines +75 to +77
// we can override all content type parsers (including application/json) in
// this plugin without affecting outer scope
addNoopContentTypeParsers(instance);
Copy link
Member Author

Choose a reason for hiding this comment

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

We are adding all known content-types as string and as regex now. From the fastify docs:

Fastify first tries to match a content-type parser with a string value before trying to find a matching RegExp.

Comment on lines +209 to +213
if (isUnaryError) {
throw errorFromJsonBytes(
responseBody,
appendHeaders(header, trailer)
appendHeaders(header, trailer),
unaryError
Copy link
Member Author

Choose a reason for hiding this comment

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

We ran into #469 with fastify, which replies with an application/json content type for a HTTP 404 when application/json was sent.

The logic was changed to raise a ConnectError for the HTTP status code when parsing fails to fix the issue. This change is applied to the transports from @bufbuild/connect-web and @bufbuild/connect-node.

@timostamm timostamm changed the title Add support for express Add support for Express Feb 21, 2023
@timostamm timostamm requested a review from smaye81 February 21, 2023 23:54
@timostamm timostamm marked this pull request as ready for review February 21, 2023 23:54
@timostamm timostamm merged commit 2f37f73 into main Feb 22, 2023
@timostamm timostamm deleted the tstamm/add-express branch February 22, 2023 08:38
@smaye81 smaye81 mentioned this pull request Feb 22, 2023
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.

Way to recover from errors caused by non-compliant response body
2 participants