Skip to content

Commit

Permalink
grpcweb-transport: fix handling responses with empty body and status …
Browse files Browse the repository at this point in the history 
…in headers (#331)
  • Loading branch information
@eKazim
eKazim authored Jul 7, 2022
1 parent 18d59ef commit 02c82bd
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 25 deletions.
12 changes: 10 additions & 2 deletions packages/grpcweb-transport/spec/grpc-web-format.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ describe('readGrpcWebResponse', () => {
expect(statusInvalid2[0]).toEqual(GrpcStatusCode.INTERNAL);
});

it('handles empty HTTP headers', function () {
const actual1 = readGrpcWebResponseHeader({}, 200, 'success');
expect(actual1).toEqual([undefined, undefined, {}]);

const actual2 = readGrpcWebResponseHeader({}, 400, 'invalid');
expect(actual2).toEqual([GrpcStatusCode.INVALID_ARGUMENT, 'invalid', {}]);
});

it('handles normal Responses', function () {
const headers = new globalThis.Headers([
['foo', 'bar'],
Expand Down Expand Up @@ -249,7 +257,7 @@ describe('readGrpcWebResponse', () => {
const input = [38, 39];
const message = createGrpcWebRequestBody(new Uint8Array(input), 'text');
const [first, ...rest] = asciiToCharCodes(message);

await next([first]);
assertFrames(frames, []);

Expand Down Expand Up @@ -384,7 +392,7 @@ describe('readGrpcWebResponse', () => {
const trailer = readGrpcWebResponseTrailer(new Uint8Array([]));
expect(trailer).toEqual([GrpcStatusCode.OK, undefined, {}])
});

it('handles success status without message', function () {
const trailer = readGrpcWebResponseTrailer(trailerFromObject({
'grpc-status': GrpcStatusCode.OK,
Expand Down
76 changes: 68 additions & 8 deletions packages/grpcweb-transport/spec/grpc-web-transport.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ describe('GrpcWebFetchTransport', () => {
afterAll(() => {
globalThis.fetch = originalFetch;
});

beforeEach(() => {
fetch = jasmine.createSpy('fetch', originalFetch);
globalThis.fetch = fetch;
Expand Down Expand Up @@ -236,7 +236,7 @@ describe('GrpcWebFetchTransport', () => {

// Tailers with OK status, stream finished
await microTaskDelay(next(getTrailerFrame(GrpcStatusCode.OK), true));

// Everything resolved
expect(spy.headers).toHaveBeenCalledTimes(1);
expect(spy.response).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -298,7 +298,39 @@ describe('GrpcWebFetchTransport', () => {
}
});

it('throws `DATA_LOSS` if it does not get a trailer', async () => {
it('throws `INTERNAL` if empty response body and no status in headers', async () => {
const { next, call } = getNextCall();
await next([], true);
try {
await call;
fail('should fail');
} catch (e) {
expect(e).toBeInstanceOf(RpcError);
expect(e.code).toBe(GrpcStatusCode[GrpcStatusCode.INTERNAL]);
}
});

it('throws `DATA_LOSS` if empty response body and OK status in headers', async () => {
const { next, call } = getNextCall({
status: 200,
statusText: 'success',
headers: new globalThis.Headers({
'some': 'header',
'content-type': 'application/grpc-web+proto',
'grpc-status': '0',
}),
});
await next([], true);
try {
await call;
fail('should fail');
} catch (e) {
expect(e).toBeInstanceOf(RpcError);
expect(e.code).toBe(GrpcStatusCode[GrpcStatusCode.DATA_LOSS]);
}
});

it('throws `DATA_LOSS` if it does not get a trailer with response', async () => {
const { next, call } = getNextCall();
await next(getDataFrame('response'), true);
try {
Expand Down Expand Up @@ -383,7 +415,7 @@ describe('GrpcWebFetchTransport', () => {
expect(e.code).toBe(GrpcStatusCode[GrpcStatusCode.INTERNAL]);
}
});

});

describe('serverStreaming()', () => {
Expand Down Expand Up @@ -421,7 +453,7 @@ describe('GrpcWebFetchTransport', () => {
afterAll(() => {
globalThis.fetch = originalFetch;
});

beforeEach(() => {
fetch = jasmine.createSpy('fetch', originalFetch);
globalThis.fetch = fetch;
Expand Down Expand Up @@ -514,7 +546,7 @@ describe('GrpcWebFetchTransport', () => {

// Tailers with OK status, stream finished
await microTaskDelay(next(getTrailerFrame(GrpcStatusCode.OK), true));

// Everything resolved
expect(spy.headers).toHaveBeenCalledTimes(1);
expect(responsesIteratorSpy).toHaveBeenCalledTimes(3);
Expand Down Expand Up @@ -578,7 +610,35 @@ describe('GrpcWebFetchTransport', () => {
}
});

it('throws `DATA_LOSS` if it does not get a trailer', async () => {
it('throws `INTERNAL` if empty response body and no status in headers', async () => {
const { next, call } = getNextCall();
await next([], true);
try {
await call;
fail('should fail');
} catch (e) {
expect(e).toBeInstanceOf(RpcError);
expect(e.code).toBe(GrpcStatusCode[GrpcStatusCode.INTERNAL]);
}
});

it('success if empty response body and OK status in headers', async () => {
const { next, call } = getNextCall({
status: 200,
statusText: 'success',
headers: new globalThis.Headers({
'some': 'header',
'content-type': 'application/grpc-web+proto',
'grpc-status': '0',
}),
});
await next([], true);
const streamResult = await call;
expect(streamResult.status.code).toBe('OK');
expect(streamResult.headers).toEqual({ some: 'header' });
});

it('throws `DATA_LOSS` if it does not get a trailer with response', async () => {
const { next, call } = getNextCall();
await next(getDataFrame('response'), true);
try {
Expand Down Expand Up @@ -642,7 +702,7 @@ describe('GrpcWebFetchTransport', () => {
expect(e.code).toBe(GrpcStatusCode[GrpcStatusCode.INTERNAL]);
}
});

});


Expand Down
16 changes: 8 additions & 8 deletions packages/grpcweb-transport/src/grpc-web-format.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ export function createGrpcWebRequestBody(message: Uint8Array, format: GrpcWebFor
* If given a fetch response, checks for fetch-specific error information
* ("type" property) and whether the "body" is null and throws a RpcError.
*/
export function readGrpcWebResponseHeader(fetchResponse: Response): [GrpcStatusCode, string | undefined, RpcMetadata];
export function readGrpcWebResponseHeader(headers: HttpHeaders, httpStatus: number, httpStatusText: string): [GrpcStatusCode, string | undefined, RpcMetadata];
export function readGrpcWebResponseHeader(headersOrFetchResponse: HttpHeaders | Response, httpStatus?: number, httpStatusText?: string): [GrpcStatusCode, string | undefined, RpcMetadata] {
export function readGrpcWebResponseHeader(fetchResponse: Response): [GrpcStatusCode | undefined, string | undefined, RpcMetadata];
export function readGrpcWebResponseHeader(headers: HttpHeaders, httpStatus: number, httpStatusText: string): [GrpcStatusCode | undefined, string | undefined, RpcMetadata];
export function readGrpcWebResponseHeader(headersOrFetchResponse: HttpHeaders | Response, httpStatus?: number, httpStatusText?: string): [GrpcStatusCode | undefined, string | undefined, RpcMetadata] {
if (arguments.length === 1) {
let fetchResponse = headersOrFetchResponse as Response;

Expand Down Expand Up @@ -113,7 +113,7 @@ export function readGrpcWebResponseHeader(headersOrFetchResponse: HttpHeaders |
responseMeta = parseMetadata(headers),
[statusCode, statusDetail] = parseStatus(headers);

if (statusCode === GrpcStatusCode.OK && !httpOk) {
if ((statusCode === undefined || statusCode === GrpcStatusCode.OK) && !httpOk) {
statusCode = httpStatusToGrpc(httpStatus!);
statusDetail = httpStatusText;
}
Expand All @@ -134,7 +134,7 @@ export function readGrpcWebResponseTrailer(data: Uint8Array): [GrpcStatusCode, s
headers = parseTrailer(data),
[code, detail] = parseStatus(headers),
meta = parseMetadata(headers);
return [code, detail, meta];
return [code ?? GrpcStatusCode.OK, detail, meta];
}


Expand Down Expand Up @@ -310,9 +310,9 @@ function parseFormat(contentType: string | undefined | null): GrpcWebFormat {
}


// returns error code on parse failure, uses OK as default code
function parseStatus(headers: HttpHeaders): [GrpcStatusCode, string | undefined] {
let code = GrpcStatusCode.OK,
// returns error code on parse failure
function parseStatus(headers: HttpHeaders): [GrpcStatusCode | undefined, string | undefined] {
let code: GrpcStatusCode | undefined,
message: string | undefined;
let m = headers['grpc-message'];
if (m !== undefined) {
Expand Down
25 changes: 18 additions & 7 deletions packages/grpcweb-transport/src/grpc-web-transport.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export class GrpcWebFetchTransport implements RpcTransport {
inputBytes = method.I.toBinary(input, opt.binaryOptions),
defHeader = new Deferred<RpcMetadata>(),
responseStream = new RpcOutputStreamController<O>(),
responseEmptyBody = true,
maybeStatus: RpcStatus | undefined,
defStatus = new Deferred<RpcStatus>(),
maybeTrailer: RpcMetadata | undefined,
Expand All @@ -109,8 +110,13 @@ export class GrpcWebFetchTransport implements RpcTransport {
.then(fetchResponse => {
let [code, detail, meta] = readGrpcWebResponseHeader(fetchResponse);
defHeader.resolve(meta);
if (code !== GrpcStatusCode.OK)
if (code != null && code !== GrpcStatusCode.OK)
throw new RpcError(detail ?? GrpcStatusCode[code], GrpcStatusCode[code], meta);
if (code != null)
maybeStatus = {
code: GrpcStatusCode[code],
detail: detail ?? GrpcStatusCode[code]
};
return fetchResponse;
})

Expand All @@ -123,6 +129,7 @@ export class GrpcWebFetchTransport implements RpcTransport {
responseStream.notifyMessage(
method.O.fromBinary(data, opt.binaryOptions)
);
responseEmptyBody = false;
break;
case GrpcWebFrame.TRAILER:
let code, detail;
Expand All @@ -137,8 +144,7 @@ export class GrpcWebFetchTransport implements RpcTransport {
})

.then(() => {

if (!maybeTrailer)
if (!maybeTrailer && !responseEmptyBody)
throw new RpcError(`missing trailers`, GrpcStatusCode[GrpcStatusCode.DATA_LOSS]);

// istanbul ignore if - this should be impossible and only here to satisfy TypeScript
Expand All @@ -150,7 +156,7 @@ export class GrpcWebFetchTransport implements RpcTransport {

responseStream.notifyComplete();
defStatus.resolve(maybeStatus);
defTrailer.resolve(maybeTrailer);
defTrailer.resolve(maybeTrailer || {});

})

Expand Down Expand Up @@ -212,8 +218,13 @@ export class GrpcWebFetchTransport implements RpcTransport {
.then(fetchResponse => {
let [code, detail, meta] = readGrpcWebResponseHeader(fetchResponse);
defHeader.resolve(meta);
if (code !== GrpcStatusCode.OK)
if (code != null && code !== GrpcStatusCode.OK)
throw new RpcError(detail ?? GrpcStatusCode[code], GrpcStatusCode[code], meta);
if (code != null)
maybeStatus = {
code: GrpcStatusCode[code],
detail: detail ?? GrpcStatusCode[code]
};
return fetchResponse;
})

Expand All @@ -240,7 +251,7 @@ export class GrpcWebFetchTransport implements RpcTransport {
})

.then(() => {
if (!maybeTrailer)
if (!maybeTrailer && maybeMessage)
throw new RpcError(`missing trailers`, GrpcStatusCode[GrpcStatusCode.DATA_LOSS]);

// istanbul ignore if - this should be impossible and only here to satisfy TypeScript
Expand All @@ -258,7 +269,7 @@ export class GrpcWebFetchTransport implements RpcTransport {
throw new RpcError(maybeStatus.detail, maybeStatus.code, maybeTrailer);

defStatus.resolve(maybeStatus);
defTrailer.resolve(maybeTrailer);
defTrailer.resolve(maybeTrailer || {});

})

Expand Down

0 comments on commit 02c82bd

Please sign in to comment.