Skip to content

Commit

Permalink
Always capture header in ConnecError (#924)
Browse files Browse the repository at this point in the history
  • Loading branch information
srikrsna-buf authored Nov 24, 2023
1 parent 311b514 commit 2993118
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 12 deletions.
2 changes: 1 addition & 1 deletion packages/connect-web-bench/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ it like a web server would usually do.

| code generator | bundle size | minified | compressed |
|----------------|-------------------:|-----------------------:|---------------------:|
| connect | 115,395 b | 50,680 b | 13,669 b |
| connect | 115,578 b | 50,754 b | 13,683 b |
| grpc-web | 414,071 b | 300,352 b | 53,255 b |
9 changes: 7 additions & 2 deletions packages/connect-web/src/connect-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ export function createConnectTransport(
async function* parseResponseBody(
body: ReadableStream<Uint8Array>,
trailerTarget: Headers,
header: Headers,
) {
const reader = createEnvelopeReadableStream(body).getReader();
let endStreamReceived = false;
Expand All @@ -271,7 +272,11 @@ export function createConnectTransport(
endStreamReceived = true;
const endStream = endStreamFromJson(data);
if (endStream.error) {
throw endStream.error;
const error = endStream.error;
header.forEach((value, key) => {
error.metadata.append(key, value);
});
throw error;
}
endStream.metadata.forEach((value, key) =>
trailerTarget.set(key, value),
Expand Down Expand Up @@ -346,7 +351,7 @@ export function createConnectTransport(
...req,
header: fRes.headers,
trailer,
message: parseResponseBody(fRes.body, trailer),
message: parseResponseBody(fRes.body, trailer, fRes.headers),
};
return res;
},
Expand Down
12 changes: 9 additions & 3 deletions packages/connect-web/src/grpc-web-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ export function createGrpcWebTransport(
if (trailer === undefined) {
throw "missing trailer";
}
validateTrailer(trailer);
validateTrailer(trailer, response.headers);
if (message === undefined) {
throw "missing message";
}
Expand Down Expand Up @@ -250,6 +250,7 @@ export function createGrpcWebTransport(
body: ReadableStream<Uint8Array>,
foundStatus: boolean,
trailerTarget: Headers,
header: Headers,
) {
const reader = createEnvelopeReadableStream(body).getReader();
if (foundStatus) {
Expand Down Expand Up @@ -279,7 +280,7 @@ export function createGrpcWebTransport(
}
trailerReceived = true;
const trailer = trailerParse(data);
validateTrailer(trailer);
validateTrailer(trailer, header);
trailer.forEach((value, key) => trailerTarget.set(key, value));
continue;
}
Expand Down Expand Up @@ -348,7 +349,12 @@ export function createGrpcWebTransport(
...req,
header: fRes.headers,
trailer,
message: parseResponseBody(fRes.body, foundStatus, trailer),
message: parseResponseBody(
fRes.body,
foundStatus,
trailer,
fRes.headers,
),
};
return res;
},
Expand Down
6 changes: 5 additions & 1 deletion packages/connect/src/protocol-connect/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,11 @@ export function createTransport(opt: CommonTransportOptions): Transport {
}
endStreamReceived = true;
if (chunk.value.error) {
throw chunk.value.error;
const error = chunk.value.error;
uRes.header.forEach((value, key) => {
error.metadata.append(key, value);
});
throw error;
}
chunk.value.metadata.forEach((value, key) =>
res.trailer.set(key, value),
Expand Down
4 changes: 2 additions & 2 deletions packages/connect/src/protocol-grpc-web/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
Code.InvalidArgument,
);
}
validateTrailer(trailer);
validateTrailer(trailer, uRes.header);
if (message === undefined) {
throw new ConnectError(
"protocol error: missing output message for unary method",
Expand Down Expand Up @@ -305,7 +305,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
);
}
trailerReceived = true;
validateTrailer(chunk.value);
validateTrailer(chunk.value, uRes.header);
chunk.value.forEach((value, key) =>
res.trailer.set(key, value),
);
Expand Down
4 changes: 2 additions & 2 deletions packages/connect/src/protocol-grpc/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
},
{ propagateDownStreamError: false },
);
validateTrailer(uRes.trailer);
validateTrailer(uRes.trailer, uRes.header);
if (message === undefined) {
throw new ConnectError(
"protocol error: missing output message for unary method",
Expand Down Expand Up @@ -245,7 +245,7 @@ export function createTransport(opt: CommonTransportOptions): Transport {
async function* (iterable) {
yield* iterable;
if (!foundStatus) {
validateTrailer(uRes.trailer);
validateTrailer(uRes.trailer, uRes.header);
}
},
{ propagateDownStreamError: true },
Expand Down
5 changes: 4 additions & 1 deletion packages/connect/src/protocol-grpc/validate-trailer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ import { findTrailerError } from "./trailer-status.js";
*
* @private Internal code, does not follow semantic versioning.
*/
export function validateTrailer(trailer: Headers): void {
export function validateTrailer(trailer: Headers, header: Headers): void {
const err = findTrailerError(trailer);
if (err) {
header.forEach((value, key) => {
err.metadata.append(key, value);
});
throw err;
}
}

0 comments on commit 2993118

Please sign in to comment.