Disregard non-JSON error response bodies

packages/connect-web-bench/README.md

@@ -10,5 +10,5 @@ it like a web server would usually do.
 
 | code generator | bundle size | minified | compressed |
 |----------------|-------------------:|-----------------------:|---------------------:|
-| connect | 113,120 b | 49,644 b | 13,266 b |
+| connect | 113,630 b | 49,881 b | 13,372 b |
 | grpc-web | 414,906 b | 301,127 b | 53,279 b |

packages/connect/src/protocol-connect/content-type.spec.ts

@@ -22,6 +22,8 @@ import {
 
 describe("parseContentType()", function () {
  it("should parse", function () {
+ expect(parseContentType("text/plain")).toBeUndefined();
+ expect(parseContentType("text/plain; charset=utf-8")).toBeUndefined();
  expect(parseContentType(contentTypeUnaryJson)).toEqual({
  stream: false,
  binary: false,

packages/connect/src/protocol-connect/transport.spec.ts

@@ -37,6 +37,7 @@ import { Code } from "../code.js";
 import {
  contentTypeStreamProto,
  contentTypeUnaryProto,
+ contentTypeUnaryJson,
 } from "./content-type.js";
 import { errorToJsonBytes } from "./error-json.js";
 import { createHandlerFactory } from "./handler-factory.js";
@@ -94,31 +95,54 @@ describe("Connect transport", function () {
  describe("against server responding with an error", function () {
  describe("for unary", function () {
  let httpRequestAborted = false;
- const t = createTransport({
- httpClient(
- request: UniversalClientRequest
- ): Promise<UniversalClientResponse> {
- request.signal?.addEventListener(
- "abort",
- () => (httpRequestAborted = true)
+ const getUnaryTransport = (opts: { contentType: string }) => {
+ return createTransport({
+ httpClient(
+ request: UniversalClientRequest
+ ): Promise<UniversalClientResponse> {
+ request.signal?.addEventListener(
+ "abort",
+ () => (httpRequestAborted = true)
+ );
+ return Promise.resolve({
+ status: 429,
+ header: new Headers({
+ "Content-Type": opts.contentType,
+ }),
+ body: createAsyncIterable([
+ errorToJsonBytes(
+ new ConnectError("foo", Code.ResourceExhausted),
+ {}
+ ),
+ ]),
+ trailer: new Headers(),
+ });
+ },
+ ...defaultTransportOptions,
+ });
+ };
+ it("should cancel the HTTP request and not parse the body", async function () {
+ const t = getUnaryTransport({ contentType: contentTypeUnaryProto });
+ try {
+ await t.unary(
+ TestService,
+ TestService.methods.unary,
+ undefined,
+ undefined,
+ undefined,
+ {}
  );
- return Promise.resolve({
- status: 429,
- header: new Headers({
- "Content-Type": contentTypeUnaryProto,
- }),
- body: createAsyncIterable([
- errorToJsonBytes(
- new ConnectError("foo", Code.ResourceExhausted),
- {}
- ),
- ]),
- trailer: new Headers(),
- });
- },
- ...defaultTransportOptions,
+ fail("expected error");
+ } catch (e) {
+ expect(e).toBeInstanceOf(ConnectError);
+ // Body should not be parsed because the Content-Type response header is not application/json
+ expect(ConnectError.from(e).code).toBe(Code.Unavailable);
+ expect(ConnectError.from(e).message).toBe("[unavailable] HTTP 429");
+ }
+ expect(httpRequestAborted).toBeTrue();
  });
- it("should cancel the HTTP request", async function () {
+ it("should cancel the HTTP request and parse the body", async function () {
+ const t = getUnaryTransport({ contentType: contentTypeUnaryJson });
  try {
  await t.unary(
  TestService,
@@ -131,6 +155,8 @@ describe("Connect transport", function () {
  fail("expected error");
  } catch (e) {
  expect(e).toBeInstanceOf(ConnectError);
+ // Body should be parsed because the Content-Type response header is application/json
+ expect(ConnectError.from(e).code).toBe(Code.ResourceExhausted);
  expect(ConnectError.from(e).message).toBe("[resource_exhausted] foo");
  }
  expect(httpRequestAborted).toBeTrue();

packages/connect/src/protocol-connect/validate-response.spec.ts

@@ -15,40 +15,54 @@
 import { MethodKind } from "@bufbuild/protobuf";
 import { validateResponse } from "./validate-response.js";
 import { ConnectError } from "../connect-error.js";
+import { Code } from "../code.js";
 
 describe("Connect validateResponse()", function () {
  describe("with unary", function () {
- const methodKind = MethodKind.Unary;
- it("should be successful for HTTP 200", function () {
- const r = validateResponse(methodKind, 200, new Headers());
+ it("should be successful for HTTP 200 with proper unary JSON content type", function () {
+ const r = validateResponse(
+ MethodKind.Unary,
+ 200,
+ new Headers({ "Content-Type": "application/json" })
+ );
  expect(r.isUnaryError).toBeFalse();
  expect(r.unaryError).toBeUndefined();
  });
  it("should return error for HTTP 204", function () {
- const r = validateResponse(methodKind, 204, new Headers());
+ const r = validateResponse(
+ MethodKind.Unary,
+ 204,
+ new Headers({ "Content-Type": "application/json" })
+ );
  expect(r.isUnaryError).toBeTrue();
  expect(r.unaryError?.message).toBe("[unknown] HTTP 204");
  });
- it("should return error for HTTP error status", function () {
- const r = validateResponse(methodKind, 400, new Headers());
- expect(r.isUnaryError).toBeTrue();
- expect(r.unaryError?.message).toBe("[invalid_argument] HTTP 400");
- });
  it("should include headers as error metadata", function () {
- const r = validateResponse(methodKind, 204, new Headers({ Foo: "Bar" }));
+ const r = validateResponse(
+ MethodKind.Unary,
+ 204,
+ new Headers({ "Content-Type": "application/json", Foo: "Bar" })
+ );
  expect(r.unaryError?.metadata.get("Foo")).toBe("Bar");
  });
- });
- describe("with streaming", function () {
- const methodKind = MethodKind.ServerStreaming;
- it("should be successful for HTTP 200", function () {
- const r = validateResponse(methodKind, 200, new Headers());
+ it("should be successful for HTTP 200 with proper unary proto content type", function () {
+ const r = validateResponse(
+ MethodKind.Unary,
+ 200,
+ new Headers({ "Content-Type": "application/proto" })
+ );
  expect(r.isUnaryError).toBeFalse();
  expect(r.unaryError).toBeUndefined();
  });
- it("should throw error for HTTP error status", function () {
+ it("should throw error for HTTP error status with binary response body", function () {
  try {
- validateResponse(methodKind, 400, new Headers());
+ validateResponse(
+ MethodKind.Unary,
+ 400,
+ new Headers({
+ "Content-Type": "application/proto",
+ })
+ );
  fail("expected error");
  } catch (e) {
  expect(e).toBeInstanceOf(ConnectError);
@@ -57,9 +71,30 @@ describe("Connect validateResponse()", function () {
  );
  }
  });
+ it("should return an error for HTTP error status if content type is JSON", function () {
+ const result = validateResponse(
+ MethodKind.Unary,
+ 400,
+ new Headers({
+ "Content-Type": "application/json",
+ })
+ );
+ expect(result.isUnaryError).toBeTrue();
+ expect(result.unaryError?.code).toBe(Code.InvalidArgument);
+ expect(result.unaryError?.message).toBe("[invalid_argument] HTTP 400");
+ });
+ });
+ describe("with streaming", function () {
  it("should include headers as error metadata", function () {
  try {
- validateResponse(methodKind, 400, new Headers({ Foo: "Bar" }));
+ validateResponse(
+ MethodKind.BiDiStreaming,
+ 400,
+ new Headers({
+ "Content-Type": "application/connect+proto",
+ Foo: "Bar",
+ })
+ );
  fail("expected error");
  } catch (e) {
  expect(e).toBeInstanceOf(ConnectError);

packages/connect/src/protocol-connect/validate-response.ts

@@ -16,6 +16,7 @@ import { MethodKind } from "@bufbuild/protobuf";
 import { Code } from "../code.js";
 import { codeFromHttpStatus } from "./http-status.js";
 import { ConnectError } from "../connect-error.js";
+import { parseContentType } from "./content-type.js";
 import { headerStreamEncoding, headerUnaryEncoding } from "./headers.js";
 import type { Compression } from "../protocol/compression.js";
 
@@ -36,13 +37,16 @@ export function validateResponse(
 ):
  | { isUnaryError: false; unaryError?: undefined }
  | { isUnaryError: true; unaryError: ConnectError } {
+ const mimeType = headers.get("Content-Type");
+ const parsedType = parseContentType(mimeType);
  if (status !== 200) {
  const errorFromStatus = new ConnectError(
  `HTTP ${status}`,
  codeFromHttpStatus(status),
  headers
  );
- if (methodKind == MethodKind.Unary) {
+ // If parsedType is defined and it is not binary, then this is a unary JSON response
+ if (methodKind == MethodKind.Unary && parsedType && !parsedType.binary) {
  return { isUnaryError: true, unaryError: errorFromStatus };
  }
  throw errorFromStatus;