diff --git a/libraries/botframework-streaming/src/namedPipe/namedPipeTransport.ts b/libraries/botframework-streaming/src/namedPipe/namedPipeTransport.ts index 2daebe1306..69f0824786 100644 --- a/libraries/botframework-streaming/src/namedPipe/namedPipeTransport.ts +++ b/libraries/botframework-streaming/src/namedPipe/namedPipeTransport.ts @@ -85,7 +85,11 @@ export class NamedPipeTransport implements ITransportSender, ITransportReceiver * @returns The buffer containing the data from the transport. */ receive(count: number): Promise { - if (this._activeReceiveResolve) { + if (!this.socket) { + throw new Error('Cannot receive data over an unavailable/null socket.'); + } else if (this.socket.destroyed) { + throw new Error('Cannot receive data over a dead/destroyed socket.'); + } else if (this._activeReceiveResolve) { throw new Error('Cannot call receive more than once before it has returned.'); } diff --git a/libraries/botframework-streaming/tests/NamedPipe.test.js b/libraries/botframework-streaming/tests/NamedPipe.test.js index 5892db99b2..762b3c8dfd 100644 --- a/libraries/botframework-streaming/tests/NamedPipe.test.js +++ b/libraries/botframework-streaming/tests/NamedPipe.test.js @@ -1,12 +1,10 @@ const assert = require('assert'); const { expect } = require('chai'); -const { expectEventually } = require('./helpers/expectEventually'); -const { NamedPipeClient, NamedPipeServer, StreamingRequest } = require('../lib'); +const { NamedPipeClient, NamedPipeServer, StreamingRequest, StreamingResponse } = require('../lib'); const { NamedPipeTransport } = require('../lib/namedPipe'); const { platform } = require('os'); const { RequestHandler } = require('../lib'); const { createNodeServer, getServerFactory } = require('../lib/utilities/createNodeServer'); - class FauxSock { constructor(contentString) { if (contentString) { @@ -166,9 +164,7 @@ describe.windowsOnly('Streaming Extensions NamedPipe Library Tests', function () expect(() => transport.close()).to.not.throw(); }); - // TODO: 2023-04-24 [hawo] #4462 The code today does not allows the receive() call to be rejected by reading a dead socket. - // The receive() call will be rejected IFF the socket is closed/error AFTER the receive() call. - it.skip('throws when reading from a dead socket', async function () { + it('throws when reading from a dead socket', async function () { const sock = new FauxSock(); sock.destroyed = true; sock.connecting = false; @@ -176,10 +172,23 @@ describe.windowsOnly('Streaming Extensions NamedPipe Library Tests', function () const transport = new NamedPipeTransport(sock, 'fakeSocket5'); expect(transport).to.be.instanceOf(NamedPipeTransport); expect(transport.isConnected).to.be.false; - (await expectEventually(transport.receive(5))).to.throw(); + expect(() => transport.receive(5)).to.throw('Cannot receive data over a dead/destroyed socket.'); expect(() => transport.close()).to.not.throw(); }); + it('throws when reading from a unavailable/null socket', async function () { + const sock = new FauxSock(); + sock.destroyed = false; + sock.connecting = false; + sock.writable = true; + const transport = new NamedPipeTransport(sock, 'fakeSocket5'); + expect(transport).to.be.instanceOf(NamedPipeTransport); + expect(() => transport.close()).to.not.throw(); + expect(transport.isConnected).to.be.false; + expect(transport.socket).to.be.null; + expect(() => transport.receive(5)).to.throw('Cannot receive data over an unavailable/null socket.'); + }); + it('can read from the socket', function () { const sock = new FauxSock(); sock.destroyed = false; @@ -258,20 +267,44 @@ describe.windowsOnly('Streaming Extensions NamedPipe Library Tests', function () expect(() => client.disconnect()).to.not.throw(); }); - // TODO: 2023-04-24 [hawo] #4462 The client.send() call will only resolve when the other side responded. - // Because the other side is not connected to anything, thus, no response is received. - // Thus, the Promise is not resolved. - it.skip('sends without throwing', function (done) { - const req = new StreamingRequest(); - req.Verb = 'POST'; - req.Path = 'some/path'; - req.setBody('Hello World!'); - client - .send(req) - .catch((err) => { - expect(err).to.be.undefined; - }) - .then(done); + it('sends without throwing', function (done) { + const response = new StreamingResponse(); + response.statusCode = 111; + response.setBody('Test body.'); + class TestRequestHandler extends RequestHandler { + constructor() { + super(); + } + processRequest(_request, _logger) { + return response; + } + } + const server = new NamedPipeServer('pipeClientTest', new TestRequestHandler()); + const client = new NamedPipeClient('pipeClientTest'); + const pingServer = () => + new Promise((resolve) => { + // Ping server before sending any information, so the server makes the connection over the socket. + client.send(new StreamingRequest()); + const interval = setInterval(() => { + if (server.isConnected) { + clearInterval(interval); + resolve(); + } + }, 100); + }); + try { + server.start(async () => { + await client.connect(); + await pingServer(); + const result = await client.send(new StreamingRequest()); + expect(result.statusCode).to.equal(response.statusCode); + server.disconnect(); + client.disconnect(); + done(); + }); + } catch (err) { + done(err); + } }); }); @@ -310,21 +343,44 @@ describe.windowsOnly('Streaming Extensions NamedPipe Library Tests', function () expect(server.isConnected).to.be.true; }); - // TODO: 2023-04-24 [hawo] #4462 The client.send() call will only resolve when the other side responded. - // Because the other side is not connected to anything, thus, no response is received. - // Thus, the Promise is not resolved. - it.skip('sends without throwing', function (done) { - const server = new NamedPipeServer('pipeA', new RequestHandler()); - expect(server).to.be.instanceOf(NamedPipeServer); - expect(() => server.start()).to.not.throw(); - const req = { verb: 'POST', path: '/api/messages', streams: [] }; - server - .send(req) - .catch((err) => { - expect(err).to.be.undefined; - }) - .then(expect(() => server.disconnect()).to.not.throw()) - .then(done); + it('sends without throwing', function (done) { + const response = new StreamingResponse(); + response.statusCode = 111; + response.setBody('Test body.'); + class TestRequestHandler extends RequestHandler { + constructor() { + super(); + } + processRequest(_request, _logger) { + return response; + } + } + const server = new NamedPipeServer('pipeServerTest'); + const client = new NamedPipeClient('pipeServerTest', new TestRequestHandler()); + const pingServer = () => + new Promise((resolve) => { + // Ping server before sending any information, so the server makes the connection over the socket. + client.send(new StreamingRequest()); + const interval = setInterval(() => { + if (server.isConnected) { + clearInterval(interval); + resolve(); + } + }, 100); + }); + try { + server.start(async () => { + await client.connect(); + await pingServer(); + const result = await server.send(new StreamingRequest()); + expect(result.statusCode).to.equal(response.statusCode); + server.disconnect(); + client.disconnect(); + done(); + }); + } catch (err) { + done(err); + } }); it('handles being disconnected', function () {