diff --git a/client/grpc-web-fake-transport/README.md b/client/grpc-web-fake-transport/README.md index 5069b956..2950b8c8 100644 --- a/client/grpc-web-fake-transport/README.md +++ b/client/grpc-web-fake-transport/README.md @@ -3,6 +3,14 @@ ## Usage +`FakeTransportBuilder` builds a `Transport` that can be configured to send preset headers, messages, and trailers for testing. + +By default the `Transport` that `FakeTranportBuilder.build()` generates will trigger all of the specified response behaviours (`.withHeaders(metadata)`, `.withMessages([msgOne, msgTwo])`, `.withTrailers(metadata)`) when the client has finished sending. + +This is usually the desired flow as all of the response behaviour is triggered only after the client has finished sending as would most commonly occur in production usage. + +However, in the case of bi-directional or other complex usage it can be helpful to use `.withManualTrigger()` to disable automatic sending of messages or headers/trailers and trigger the sending manually using `sendHeaders()`, `sendMessages()` and `sendTrailers()`. + ### With Service Stubs generated by ts-protoc-gen ```typescript import { FakeTransportBuilder } from '@improbable-eng/grpc-web-fake-transport'; @@ -34,6 +42,30 @@ grpc.invoke(PingService.DoPing, { }); ``` +### With `withManualTrigger()` +```typescript +import { grpc } from '@improbable-eng/grpc-web'; +import { FakeTransportBuilder } from '@improbable-eng/grpc-web-fake-transport'; + +const fakeTransport = new FakeTransportBuilder() + .withManualTrigger() + .withHeaders(new grpc.Metadata({ headerKey: "value" })) + .withMessages([ new PingResponse() ]) + .withTrailers(new grpc.Metadata({ trailerKey: "value" })) + .build(); + +grpc.invoke(PingService.DoPing, { + host: "https://example.com", + transport: fakeTransport, + /* ... */ +}); + +// Manually trigger the response behaviours +fakeTransport.sendHeaders(); +fakeTransport.sendMessages(); +fakeTransport.sendTrailers(); +``` + Alternatively replace the Default Transport when initialising your tests: ```typescript import { grpc } from "@improbable-eng/grpc-web"; @@ -41,4 +73,4 @@ import { NodeHttpTransport } from "@improbable-eng/grpc-web-fake-transport"; // Do this first, before you make any grpc requests! grpc.setDefaultTransport(fakeTransport); -``` \ No newline at end of file +``` diff --git a/client/grpc-web-fake-transport/src/index.spec.ts b/client/grpc-web-fake-transport/src/index.spec.ts index a4b5e1ca..a8e102dd 100644 --- a/client/grpc-web-fake-transport/src/index.spec.ts +++ b/client/grpc-web-fake-transport/src/index.spec.ts @@ -115,7 +115,9 @@ describe("FakeTransportBuilder", () => { const expectedBytes = frameRequest(req); const transport = new FakeTransportBuilder() - .withMessageListener(messageSpy) + .withMessageListener(messageBytes => { + messageSpy(messageBytes); + }) .build(); doPingStreamRequest(transport, [ req ], () => { @@ -227,7 +229,7 @@ describe("FakeTransportBuilder", () => { }); describe("manual trigger", () => { - it("should allow the consumer to control the lifecycle of the server response", done => { + it("should allow the consumer to control the lifecycle of the server response", () => { const onHeadersSpy = jest.fn(); const onMessageSpy = jest.fn(); const onEndSpy = jest.fn(); @@ -258,30 +260,19 @@ describe("FakeTransportBuilder", () => { transport.sendHeaders(); - // Note that we must defer the assertions as the grpc-web package detaches all callbacks, so they - // will not run immediately. - setTimeout(() => { - expect(onHeadersSpy).toHaveBeenCalled(); - expect(onMessageSpy).not.toHaveBeenCalled(); - expect(onEndSpy).not.toHaveBeenCalled(); - - transport.sendMessages(); - setTimeout(() => { - expect(onHeadersSpy).toHaveBeenCalled(); - expect(onMessageSpy).toHaveBeenCalled(); - expect(onEndSpy).not.toHaveBeenCalled(); - - transport.sendTrailers(); - setTimeout(() => { - expect(onHeadersSpy).toHaveBeenCalled(); - expect(onMessageSpy).toHaveBeenCalled(); - expect(onEndSpy).toHaveBeenCalled(); - - done(); - }, 0); - }, 0); - - }, 0); + expect(onHeadersSpy).toHaveBeenCalled(); + expect(onMessageSpy).not.toHaveBeenCalled(); + expect(onEndSpy).not.toHaveBeenCalled(); + + transport.sendMessages(); + expect(onHeadersSpy).toHaveBeenCalled(); + expect(onMessageSpy).toHaveBeenCalled(); + expect(onEndSpy).not.toHaveBeenCalled(); + + transport.sendTrailers(); + expect(onHeadersSpy).toHaveBeenCalled(); + expect(onMessageSpy).toHaveBeenCalled(); + expect(onEndSpy).toHaveBeenCalled(); }); }); @@ -352,4 +343,4 @@ describe("FakeTransportBuilder", () => { } client.finishSend(); } -}); \ No newline at end of file +}); diff --git a/client/grpc-web-fake-transport/src/index.ts b/client/grpc-web-fake-transport/src/index.ts index 1fcac86a..d41525e5 100644 --- a/client/grpc-web-fake-transport/src/index.ts +++ b/client/grpc-web-fake-transport/src/index.ts @@ -277,9 +277,6 @@ export class FakeTransportBuilder { if (mock.headersListener) { mock.headersListener(metadata); } - if (mock.autoTrigger) { - triggers.sendAll(); - } }, sendMessage: (msgBytes: ArrayBufferView) => { if (mock.messageListener) { @@ -290,6 +287,9 @@ export class FakeTransportBuilder { if (mock.finishSendListener) { mock.finishSendListener(); } + if (mock.autoTrigger) { + triggers.sendAll(); + } }, cancel: () => { if (mock.cancelListener) { @@ -308,4 +308,4 @@ export interface TriggerableTransport extends grpc.TransportFactory { sendMessages(): boolean; sendTrailers(): boolean; sendAll(): void; -} \ No newline at end of file +} diff --git a/client/grpc-web/src/client.ts b/client/grpc-web/src/client.ts index 15a54510..1f53e871 100644 --- a/client/grpc-web/src/client.ts +++ b/client/grpc-web/src/client.ts @@ -2,7 +2,6 @@ import {Metadata} from "./metadata"; import {ChunkParser, Chunk, ChunkType} from "./ChunkParser"; import {Code, httpStatusToCode} from "./Code"; import {debug} from "./debug"; -import detach from "./detach"; import {Transport, TransportFactory, makeDefaultTransport} from "./transports/Transport"; import {MethodDefinition} from "./service"; import {frameRequest} from "./util"; @@ -202,10 +201,14 @@ class GrpcClient { - detach(() => { - if (this.closed) return; + if (this.closed) return; + try { callback(code, message, trailers); - }); + } catch (e) { + setTimeout(() => { + throw e; + }) + } }); } @@ -213,9 +216,13 @@ class GrpcClient { - detach(() => { + try { callback(headers); - }); + } catch (e) { + setTimeout(() => { + throw e; + }) + } }); } @@ -224,10 +231,14 @@ class GrpcClient { - detach(() => { - if (this.closed) return; + if (this.closed) return; + try { callback(code, msg, trailers); - }); + } catch (e) { + setTimeout(() => { + throw e; + }) + } }); } @@ -235,10 +246,14 @@ class GrpcClient { - detach(() => { - if (this.closed) return; + if (this.closed) return; + try { callback(res); - }); + } catch (e) { + setTimeout(() => { + throw e; + }) + } }); } diff --git a/client/grpc-web/src/detach.ts b/client/grpc-web/src/detach.ts index 1513302d..ef6a5d04 100644 --- a/client/grpc-web/src/detach.ts +++ b/client/grpc-web/src/detach.ts @@ -1,43 +1,3 @@ -// awaitingExecution is null if there is no current timer -let awaitingExecution: Array<() => void> | null = null; - -function runCallbacks() { - if (awaitingExecution) { - // Use a new reference to the awaitingExecution array to allow callbacks to add to the "new" awaitingExecution array - const thisCallbackSet = awaitingExecution; - awaitingExecution = null; - for (let i = 0; i < thisCallbackSet.length; i++) { - try { - thisCallbackSet[i](); - } catch (e) { - if (awaitingExecution === null) { - awaitingExecution = []; - setTimeout(() => { - runCallbacks(); - }, 0); - } - // Add the remaining callbacks to the array so that they can be invoked on the next pass - for (let k = thisCallbackSet.length - 1; k > i; k--) { - awaitingExecution.unshift(thisCallbackSet[k]); - } - // rethrow the error - throw e; - } - } - } -} - -// detach executes the callbacks in the order they are added with no context - this is used to avoid errors thrown -// in user callbacks being caught by handlers such as fetch's catch. This function is necessary as setTimeout in -// Safari is prone to switching the order of execution of setTimeout(0). export default function detach(cb: () => void) { - if (awaitingExecution !== null) { - // there is a timer running, add to the list and this function will be executed with that existing timer - awaitingExecution.push(cb); - return; - } - awaitingExecution = [cb]; - setTimeout(() => { - runCallbacks(); - }, 0); + cb(); } diff --git a/client/grpc-web/src/transports/http/fetch.ts b/client/grpc-web/src/transports/http/fetch.ts index 29c76b0e..1dfc3e0b 100644 --- a/client/grpc-web/src/transports/http/fetch.ts +++ b/client/grpc-web/src/transports/http/fetch.ts @@ -1,7 +1,6 @@ import {Metadata} from "../../metadata"; import {Transport, TransportFactory, TransportOptions} from "../Transport"; import {debug} from "../../debug"; -import detach from "../../detach"; type Omit = Pick> export type FetchTransportInit = Omit; @@ -44,14 +43,10 @@ class Fetch implements Transport { this.reader.read() .then((result: { done: boolean, value: Uint8Array }) => { if (result.done) { - detach(() => { - this.options.onEnd(); - }); + this.options.onEnd(); return res; } - detach(() => { - this.options.onChunk(result.value); - }); + this.options.onChunk(result.value); this.pump(this.reader, res); return; }) @@ -62,9 +57,7 @@ class Fetch implements Transport { } this.cancelled = true; this.options.debug && debug("Fetch.catch", err.message); - detach(() => { - this.options.onEnd(err); - }); + this.options.onEnd(err); }); } @@ -77,9 +70,7 @@ class Fetch implements Transport { signal: this.controller && this.controller.signal }).then((res: Response) => { this.options.debug && debug("Fetch.response", res); - detach(() => { - this.options.onHeaders(new Metadata(res.headers as any), res.status); - }); + this.options.onHeaders(new Metadata(res.headers as any), res.status); if (res.body) { this.pump(res.body.getReader(), res) return; @@ -92,9 +83,7 @@ class Fetch implements Transport { } this.cancelled = true; this.options.debug && debug("Fetch.catch", err.message); - detach(() => { - this.options.onEnd(err); - }); + this.options.onEnd(err); }); } diff --git a/client/grpc-web/src/transports/http/xhr.ts b/client/grpc-web/src/transports/http/xhr.ts index 2f82a689..80756a96 100644 --- a/client/grpc-web/src/transports/http/xhr.ts +++ b/client/grpc-web/src/transports/http/xhr.ts @@ -1,7 +1,6 @@ import {Metadata} from "../../metadata"; import {Transport, TransportFactory, TransportOptions} from "../Transport"; import {debug} from "../../debug"; -import detach from "../../detach"; import {detectMozXHRSupport, detectXHROverrideMimeTypeSupport} from "./xhrUtil"; export interface XhrTransportInit { @@ -37,24 +36,18 @@ export class XHR implements Transport { const rawText = this.xhr.response.substr(this.index); this.index = this.xhr.response.length; const asArrayBuffer = stringToArrayBuffer(rawText); - detach(() => { - this.options.onChunk(asArrayBuffer); - }); + this.options.onChunk(asArrayBuffer); } onLoadEvent() { this.options.debug && debug("XHR.onLoadEvent"); - detach(() => { - this.options.onEnd(); - }); + this.options.onEnd(); } onStateChange() { this.options.debug && debug("XHR.onStateChange", this.xhr.readyState); if (this.xhr.readyState === XMLHttpRequest.HEADERS_RECEIVED) { - detach(() => { - this.options.onHeaders(new Metadata(this.xhr.getAllResponseHeaders()), this.xhr.status); - }); + this.options.onHeaders(new Metadata(this.xhr.getAllResponseHeaders()), this.xhr.status); } } @@ -85,9 +78,7 @@ export class XHR implements Transport { xhr.addEventListener("loadend", this.onLoadEvent.bind(this)); xhr.addEventListener("error", (err: ErrorEvent) => { this.options.debug && debug("XHR.error", err); - detach(() => { - this.options.onEnd(err.error); - }); + this.options.onEnd(err.error); }); } @@ -114,9 +105,7 @@ export class MozChunkedArrayBufferXHR extends XHR { onProgressEvent() { const resp = this.xhr.response; this.options.debug && debug("MozXHR.onProgressEvent: ", new Uint8Array(resp)); - detach(() => { - this.options.onChunk(new Uint8Array(resp)); - }); + this.options.onChunk(new Uint8Array(resp)); } } diff --git a/client/grpc-web/src/transports/websocket/websocket.ts b/client/grpc-web/src/transports/websocket/websocket.ts index 8bb8b389..ff1c8306 100644 --- a/client/grpc-web/src/transports/websocket/websocket.ts +++ b/client/grpc-web/src/transports/websocket/websocket.ts @@ -1,7 +1,6 @@ import {Metadata} from "../../metadata"; import {Transport, TransportFactory, TransportOptions} from "../Transport"; import {debug} from "../../debug"; -import detach from "../../detach"; import {encodeASCII} from "../../ChunkParser"; enum WebsocketSignal { @@ -68,9 +67,7 @@ function websocketRequest(options: TransportOptions): Transport { ws.onclose = function (closeEvent) { options.debug && debug("websocketRequest.onclose", closeEvent); - detach(() => { - options.onEnd(); - }); + options.onEnd(); }; ws.onerror = function (error) { @@ -78,17 +75,13 @@ function websocketRequest(options: TransportOptions): Transport { }; ws.onmessage = function (e) { - detach(() => { - options.onChunk(new Uint8Array(e.data)); - }); + options.onChunk(new Uint8Array(e.data)); }; }, cancel: () => { options.debug && debug("websocket.abort"); - detach(() => { - ws.close(); - }); + ws.close(); } }; } diff --git a/integration_test/ts/src/detach.spec.ts b/integration_test/ts/src/detach.spec.ts deleted file mode 100644 index 2c3b4f75..00000000 --- a/integration_test/ts/src/detach.spec.ts +++ /dev/null @@ -1,131 +0,0 @@ -import {assert} from "chai"; -import detach from "../../../client/grpc-web/src/detach"; -import {UncaughtExceptionListener} from "./util"; -import { conditionallyRunTestSuite, SuiteEnum } from "../suiteUtils"; - -conditionallyRunTestSuite(SuiteEnum.detach, () => { - describe("basic execution ordering", () => { - it("should invoke a function", (done) => { - detach(() => { - done(); - }); - }); - - it("should invoke multiple functions in the order they are added", (done) => { - let index = 0; - detach(() => { - assert.equal(index, 0); - index++; - }); - detach(() => { - assert.equal(index, 1); - index++; - }); - detach(() => { - assert.equal(index, 2); - index++; - done(); - }); - }); - - it("should invoke multiple functions in the order they are added after the current context", (done) => { - let index = 0; - detach(() => { - assert.equal(index, 5); - index++; - }); - detach(() => { - assert.equal(index, 6); - index++; - }); - detach(() => { - assert.equal(index, 7); - index++; - done(); - }); - index = 5; // This should be run before the first callback - }); - }); - - describe("exception handling", () => { - let uncaughtHandler: UncaughtExceptionListener; - beforeEach(() => { - uncaughtHandler = new UncaughtExceptionListener(); - uncaughtHandler.attach(); - }); - - afterEach(() => { - uncaughtHandler.detach(); - }); - - it("should invoke remaining function when exceptions are thrown", (done) => { - let index = 0; - detach(() => { - assert.equal(index, 0); - index++; - }); - detach(() => { - assert.equal(index, 1); - index++; - throw new Error("Second callback threw error"); - }); - detach(() => { - assert.equal(index, 2); - index++; - throw new Error("Third callback threw error"); - }); - detach(() => { - uncaughtHandler.detach(); - assert.equal(index, 3); - index++; - const exceptionsCaught = uncaughtHandler.getMessages(); - assert.lengthOf(exceptionsCaught, 2); - assert.include(exceptionsCaught[0], "Second callback threw error"); - assert.include(exceptionsCaught[1], "Third callback threw error"); - done(); - }); - }); - - // This test is weakly-held. It is here to detect changes to execution ordering with other contexts, but detach - // should not be used in a manner that relies on this behaviour. - it("should invoke remaining function when exceptions are thrown", (done) => { - let index = 0; - detach(() => { - assert.equal(index, 0); - index++; - }); - detach(() => { - assert.equal(index, 1); - index++; - throw new Error("Second callback threw error"); - }); - // setTimeout below will be called between the above and below functions - detach(() => { - assert.equal(index, 3); - index++; - setTimeout(() => { - assert.equal(index, 4); - index++; - }, 0); - throw new Error("Third callback threw error"); - }); - detach(() => { - uncaughtHandler.detach(); - assert.equal(index, 5); - index++; - const exceptionsCaught = uncaughtHandler.getMessages(); - assert.lengthOf(exceptionsCaught, 2); - assert.include(exceptionsCaught[0], "Second callback threw error"); - assert.include(exceptionsCaught[1], "Third callback threw error"); - done(); - }); - - // This function will be called after the first exception is thrown. Throwing exceptions causes the subsequent - // callbacks to be run in a separate, subsequent timeout. - setTimeout(() => { - assert.equal(index, 2); - index++; - }, 0); - }); - }); -}); \ No newline at end of file diff --git a/integration_test/ts/src/spec.ts b/integration_test/ts/src/spec.ts index b3efacc5..00f45a93 100644 --- a/integration_test/ts/src/spec.ts +++ b/integration_test/ts/src/spec.ts @@ -3,7 +3,6 @@ import "./client.websocket.spec"; import "./invoke.spec"; import "./unary.spec"; import "./cancellation.spec"; -import "./detach.spec"; import "./ChunkParser.spec"; -jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; \ No newline at end of file +jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; diff --git a/integration_test/ts/suiteUtils.ts b/integration_test/ts/suiteUtils.ts index 792830de..6339defd 100644 --- a/integration_test/ts/suiteUtils.ts +++ b/integration_test/ts/suiteUtils.ts @@ -5,7 +5,6 @@ export enum SuiteEnum { unary, ChunkParser, cancellation, - detach, } type enumMap = {[key: string]: number} & {[key: number]: string};