V2: Require HTTP/2 for the gRPC transport (#1279)

connectrpc · Oct 17, 2024 · f6c9fac · f6c9fac
1 parent d81d475
commit f6c9fac
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 66 deletions.
diff --git a/packages/connect-node/README.md b/packages/connect-node/README.md
@@ -38,7 +38,6 @@ import { ElizaService } from "./gen/eliza_connect.js";
 + // A transport for clients using the gRPC protocol with Node.js `http2` module
 + const transport = createGrpcTransport({
 +   baseUrl: "https://demo.connectrpc.com",
-+   httpVersion: "2"
 + });
 
 const client = createClient(ElizaService, transport);
@@ -123,7 +122,6 @@ import { ElizaService } from "./gen/eliza_connect.js";
 
 const transport = createGrpcTransport({
   baseUrl: "http://localhost:8080",
-  httpVersion: "2",
 });
 
 const client = createClient(ElizaService, transport);

diff --git a/packages/connect-node/src/connect-transport.ts b/packages/connect-node/src/connect-transport.ts
@@ -21,8 +21,10 @@ import type {
 import type { Interceptor, Transport } from "@connectrpc/connect";
 import type { Compression } from "@connectrpc/connect/protocol";
 import { createTransport } from "@connectrpc/connect/protocol-connect";
-import type { NodeTransportOptions } from "./node-transport-options.js";
-import { validateNodeTransportOptions } from "./node-transport-options.js";
+import {
+  type NodeTransportOptions,
+  validateNodeTransportOptions,
+} from "./node-transport-options.js";
 
 /**
  * Options used to configure the Connect transport.
@@ -128,7 +130,7 @@ export type ConnectTransportOptions = NodeTransportOptions & {
 };
 
 /**
- * Create a Transport for the Connect protocol using the Node.js `http`, `http2`,
+ * Create a Transport for the Connect protocol using the Node.js `http`, `https`,
  * or `http2` module.
  */
 export function createConnectTransport(

diff --git a/packages/connect-node/src/grpc-transport.spec.ts b/packages/connect-node/src/grpc-transport.spec.ts
@@ -0,0 +1,40 @@
+// Copyright 2021-2024 The Connect Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { createGrpcTransport } from "./grpc-transport.js";
+
+describe("createGrpcTransport()", function () {
+  it("should take just baseUrl", function () {
+    const t = createGrpcTransport({
+      baseUrl: "https://example.com",
+    });
+    expect(t).toBeDefined();
+  });
+  it("should raise type error for httpVersion: 2", function () {
+    const t = createGrpcTransport({
+      // @ts-expect-error TS2353: Object literal may only specify known properties, and httpVersion does not exist in type GrpcTransportOptions
+      httpVersion: "2",
+      baseUrl: "https://example.com",
+    });
+    expect(t).toBeDefined();
+  });
+  it("should raise type error for httpVersion: 1.1", function () {
+    const t = createGrpcTransport({
+      // @ts-expect-error TS2353: Object literal may only specify known properties, and httpVersion does not exist in type GrpcTransportOptions
+      httpVersion: "1.1",
+      baseUrl: "https://example.com",
+    });
+    expect(t).toBeDefined();
+  });
+});
diff --git a/packages/connect-node/src/grpc-transport.ts b/packages/connect-node/src/grpc-transport.ts
@@ -21,15 +21,17 @@ import type {
   JsonReadOptions,
   JsonWriteOptions,
 } from "@bufbuild/protobuf";
-import type { NodeTransportOptions } from "./node-transport-options.js";
-import { validateNodeTransportOptions } from "./node-transport-options.js";
+import {
+  type NodeHttp2TransportOptions,
+  validateNodeTransportOptions,
+} from "./node-transport-options.js";
 
 /**
- * Options used to configure the gRPC-web transport.
+ * Options used to configure the gRPC transport.
  *
  * See createGrpcTransport().
  */
-export type GrpcTransportOptions = NodeTransportOptions & {
+export type GrpcTransportOptions = NodeHttp2TransportOptions & {
   /**
    * Base URI for all HTTP requests.
    *
@@ -123,9 +125,13 @@ export type GrpcTransportOptions = NodeTransportOptions & {
 };
 
 /**
- * Create a Transport for the gRPC protocol using the Node.js `http`, `http2`,
- * or `http2` module.
+ * Create a Transport for the gRPC protocol using the Node.js `http2` module.
  */
 export function createGrpcTransport(options: GrpcTransportOptions): Transport {
-  return createTransport(validateNodeTransportOptions(options));
+  return createTransport(
+    validateNodeTransportOptions({
+      ...options,
+      httpVersion: "2",
+    }),
+  );
 }
diff --git a/packages/connect-node/src/grpc-web-transport.ts b/packages/connect-node/src/grpc-web-transport.ts
@@ -21,8 +21,10 @@ import type {
   JsonReadOptions,
   JsonWriteOptions,
 } from "@bufbuild/protobuf";
-import type { NodeTransportOptions } from "./node-transport-options.js";
-import { validateNodeTransportOptions } from "./node-transport-options.js";
+import {
+  type NodeTransportOptions,
+  validateNodeTransportOptions,
+} from "./node-transport-options.js";
 
 /**
  * Options used to configure the gRPC-web transport.
@@ -124,7 +126,7 @@ export type GrpcWebTransportOptions = NodeTransportOptions & {
 
 /**
  * Create a Transport for the gRPC-web protocol using the Node.js `http`,
- * `http2`, or `http2` module.
+ * `https`, or `http2` module.
  */
 export function createGrpcWebTransport(
   options: GrpcWebTransportOptions,

diff --git a/packages/connect-node/src/node-readme.spec.ts b/packages/connect-node/src/node-readme.spec.ts
@@ -17,7 +17,6 @@ import {
   createContextKey,
   createContextValues,
   createClient,
-  createRouterTransport,
 } from "@connectrpc/connect";
 import type { ConnectRouter } from "@connectrpc/connect";
 import { createWritableIterable } from "@connectrpc/connect/protocol";
@@ -70,25 +69,6 @@ describe("node readme", function () {
     expect(sentence).toBeDefined();
   });
 
-  it("createRouterTransport()", async function () {
-    // A transport for clients using the in-memory createRouterTransport
-    const transport = createRouterTransport(
-      ({ service }) => {
-        service(ElizaService, {
-          say: async () => ({
-            sentence: "server response",
-          }),
-        });
-      },
-      {
-        transport: optionsHttp1,
-      },
-    );
-    const client = createClient(ElizaService, transport);
-    const { sentence } = await client.say({ sentence: "I feel happy." });
-    expect(sentence).toBeDefined();
-  });
-
   it("should work as well", async function () {
     let port = -1;
 
@@ -114,7 +94,6 @@ describe("node readme", function () {
     async function runClient() {
       const transport = createGrpcTransport({
         baseUrl: `http://localhost:${port}`,
-        httpVersion: "2",
       });
       const client = createClient(ElizaService, transport);
       const res = await client.say({ sentence: "I feel happy." });
@@ -157,7 +136,6 @@ describe("node readme", function () {
     async function runClient() {
       const transport = createGrpcTransport({
         baseUrl: `http://localhost:${port}`,
-        httpVersion: "2",
       });
       const client = createClient(ElizaService, transport);
       const res = await client.say(
@@ -204,7 +182,6 @@ describe("node readme", function () {
     async function runClient() {
       const transport = createGrpcTransport({
         baseUrl: `http://localhost:${port}`,
-        httpVersion: "2",
       });
       const client = createClient(ElizaService, transport);
       const req =

diff --git a/packages/connect-node/src/node-transport-options.ts b/packages/connect-node/src/node-transport-options.ts
@@ -27,38 +27,43 @@ import * as http from "http";
 import * as https from "https";
 
 /**
- * Options specific to Node.js client transports.
+ * Options specific to Node.js client transports that support HTTP/2 or HTTP 1.1.
  */
 export type NodeTransportOptions =
-  | {
-      httpVersion: "1.1";
-      /**
-       * Options passed to the request() call of the Node.js built-in
-       * http or https module.
-       */
-      nodeOptions?:
-        | Omit<http.RequestOptions, "signal">
-        | Omit<https.RequestOptions, "signal">;
-    }
-  | ({
-      httpVersion: "2";
+  | (NodeHttp1TransportOptions & { httpVersion: "1.1" })
+  | (NodeHttp2TransportOptions & { httpVersion: "2" });
+
+/**
+ * Options specific to Node.js client transports over HTTP/2.
+ */
+export type NodeHttp2TransportOptions = {
+  /**
+   * A manager for the HTTP/2 connection of the transport.
+   *
+   * Providing this option makes nodeOptions as well as the HTTP/2 session
+   * options (pingIntervalMs et cetera) ineffective.
+   */
+  sessionManager?: NodeHttp2ClientSessionManager;
 
-      /**
-       * A manager for the HTTP/2 connection of the transport.
-       *
-       * Providing this option makes nodeOptions as well as the HTTP/2 session
-       * options (pingIntervalMs et cetera) ineffective.
-       */
-      sessionManager?: NodeHttp2ClientSessionManager;
+  /**
+   * Options passed to the connect() call of the Node.js built-in
+   * http2 module.
+   */
+  nodeOptions?: http2.ClientSessionOptions | http2.SecureClientSessionOptions;
+} & Http2SessionOptions;
 
-      /**
-       * Options passed to the connect() call of the Node.js built-in
-       * http2 module.
-       */
-      nodeOptions?:
-        | http2.ClientSessionOptions
-        | http2.SecureClientSessionOptions;
-    } & Http2SessionOptions);
+/**
+ * Options specific to Node.js client transports over HTTP 1.1.
+ */
+type NodeHttp1TransportOptions = {
+  /**
+   * Options passed to the request() call of the Node.js built-in
+   * http or https module.
+   */
+  nodeOptions?:
+    | Omit<http.RequestOptions, "signal">
+    | Omit<https.RequestOptions, "signal">;
+};
 
 /**
  * Asserts that the options are within sane limits, and returns default values