Skip to content

Commit

Permalink
cherrypick: changes for 4.13.5 (#3711)
Browse files Browse the repository at this point in the history
* fix: named pipe issues (#3627)

* fix: named pipe issues

- Remove unnecessary autoReconnect logic
- Simplify connection management
- Properly support client reconnection to incoming/outgoing servers

* fix: PR comments, failing tests

* fix: "lock" sockets to mitigate split brain

* test: no split brain

* fix: reject listening only if server is listening

* fix: naming for clarity

* fix: clarify onListen doc string

* feat: retry helper, use for named pipes

* fix: handle retry edge cases

* bump: adal-node to 0.2.2 (#3705)

Fixes #3704

* fix: ignore node_modules declarative assets (#3709)

See microsoft/BotFramework-Composer#7916 for details.
  • Loading branch information
joshgummersall authored May 27, 2021
1 parent d2b0197 commit 0ee95d2
Show file tree
Hide file tree
Showing 16 changed files with 329 additions and 229 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,20 @@
// Licensed under the MIT License.

import { Configuration } from './configuration';
import { ComponentDeclarativeTypes, FolderResourceProvider, ResourceExplorer } from 'botbuilder-dialogs-declarative';
import { ComponentDeclarativeTypes, ResourceExplorer } from 'botbuilder-dialogs-declarative';
import { ok } from 'assert';

export class ConfigurationResourceExporer extends ResourceExplorer {
private readonly folderResourceProvider: FolderResourceProvider;

constructor(configuration: Configuration, declarativeTypes: ComponentDeclarativeTypes[]) {
super({ declarativeTypes });

const applicationRoot = configuration.string(['applicationRoot']);
ok(applicationRoot);

this.folderResourceProvider = new FolderResourceProvider(
this,
this.addFolders(
applicationRoot,
true,
['node_modules'], // Composer copies to `dialogs/imported` so `node_modules` will contain dupes
configuration.string(['NODE_ENV']) === 'dev' // watch in dev only!
);

this.addResourceProvider(this.folderResourceProvider);
}
}
1 change: 1 addition & 0 deletions libraries/botbuilder-stdlib/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export * as assertExt from './assertExt';
export * from './types';
export { delay } from './delay';
export { maybeCast } from './maybeCast';
export { retry } from './retry';
41 changes: 41 additions & 0 deletions libraries/botbuilder-stdlib/src/retry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

/**
* Retry a given promise with gradually increasing delay.
*
* @param promise a function that returns a promise to retry
* @param maxRetries the maximum number of times to retry
* @param initialDelay the initial value to delay before retrying (in milliseconds)
* @returns a promise resolving to the result of the promise from the promise generating function, or undefined
*/
export async function retry<T>(
promise: (n: number) => Promise<T>,
maxRetries: number,
initialDelay = 500
): Promise<T | undefined> {
let delay = initialDelay,
n = 1,
maybeError: Error | undefined;

// Take care of negative or zero
maxRetries = Math.max(maxRetries, 1);

while (n <= maxRetries) {
try {
// Note: return await intentional so we can catch errors
return await promise(n);
} catch (err) {
maybeError = err;

await new Promise((resolve) => setTimeout(resolve, delay));

delay *= n;
n++;
}
}

if (maybeError) {
throw maybeError;
}
}
41 changes: 41 additions & 0 deletions libraries/botbuilder-stdlib/tests/retry.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

const assert = require('assert');
const sinon = require('sinon');
const { retry } = require('../');

describe('retry', function () {
it('succeeds on first try', async function () {
const fake = sinon.fake((n) => Promise.resolve(n));
assert.strictEqual(await retry(fake, 3, 0), 1);
assert.strictEqual(fake.callCount, 1);
});

it('handles zero retries', async function () {
const fake = sinon.fake((n) => Promise.resolve(n));
assert.strictEqual(await retry(fake, 0, 0), 1);
assert.strictEqual(fake.callCount, 1);
});

it('handles negative retries', async function () {
const fake = sinon.fake((n) => Promise.resolve(n));
assert.strictEqual(await retry(fake, -10, 0), 1);
assert.strictEqual(fake.callCount, 1);
});

it('succeeds eventually', async function () {
const fake = sinon.fake((n) => (n < 3 ? Promise.reject() : Promise.resolve(10)));
assert.strictEqual(await retry(fake, 3, 0), 10);
assert.strictEqual(fake.callCount, 3);
});

it('yields error if never succeeds', async function () {
const fake = sinon.fake(() => Promise.reject(new Error('oh no')));
await assert.rejects(retry(fake, 3, 0), {
name: 'Error',
message: 'oh no',
});
assert.strictEqual(fake.callCount, 3);
});
});
18 changes: 12 additions & 6 deletions libraries/botbuilder/src/botFrameworkAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ import {

import { BotFrameworkHttpAdapter } from './botFrameworkHttpAdapter';
import { BotLogic, ConnectorClientBuilder, Emitter, Request, Response, WebRequest, WebResponse } from './interfaces';
import { delay } from 'botbuilder-stdlib';
import { delay, retry } from 'botbuilder-stdlib';
import { userAgentPolicy } from '@azure/ms-rest-js';
import { validateAndFixActivity } from './activityValidator';

Expand Down Expand Up @@ -1835,12 +1835,17 @@ export class BotFrameworkAdapter

/**
* Connects the handler to a Named Pipe server and begins listening for incoming requests.
* @param pipeName The name of the named pipe to use when creating the server.
*
* @param logic The logic that will handle incoming requests.
* @param pipeName The name of the named pipe to use when creating the server.
* @param retryCount Number of times to attempt to bind incoming and outgoing pipe
* @param onListen Optional callback that fires once when server is listening on both incoming and outgoing pipe
*/
public async useNamedPipe(
logic: (context: TurnContext) => Promise<any>,
pipeName: string = defaultPipeName
pipeName = defaultPipeName,
retryCount = 7,
onListen?: () => void
): Promise<void> {
if (!logic) {
throw new Error('Bot logic needs to be provided to `useNamedPipe`');
Expand All @@ -1862,7 +1867,8 @@ export class BotFrameworkAdapter
}

this.logic = logic;
await this.startNamedPipeServer(pipeName);

await retry(() => this.startNamedPipeServer(pipeName, onListen), retryCount);
}

/**
Expand Down Expand Up @@ -1900,12 +1906,12 @@ export class BotFrameworkAdapter
await this.startWebSocket(nodeWebSocket);
}

private async startNamedPipeServer(pipeName: string): Promise<void> {
private async startNamedPipeServer(pipeName: string, onListen?: () => void): Promise<void> {
this.namedPipeName = pipeName;
this.streamingServer = new NamedPipeServer(pipeName, this);

try {
await this.streamingServer.start();
await this.streamingServer.start(onListen);
} finally {
this.namedPipeName = undefined;
}
Expand Down
2 changes: 1 addition & 1 deletion libraries/botframework-connector/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"@azure/ms-rest-js": "1.9.1",
"@types/jsonwebtoken": "7.2.8",
"@types/node": "^10.17.27",
"adal-node": "0.2.1",
"adal-node": "0.2.2",
"base64url": "^3.0.0",
"botframework-schema": "4.1.6",
"cross-fetch": "^3.0.5",
Expand Down
9 changes: 7 additions & 2 deletions libraries/botframework-streaming/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,13 @@
"clean": "rimraf _ts3.4 es5 lib",
"lint": "eslint . --ext .js,.ts",
"postbuild": "downlevel-dts lib _ts3.4/lib --checksum",
"test": "yarn build && nyc mocha tests/",
"test:compat": "api-extractor run --verbose"
"test": "npm-run-all build test:mocha",
"test:compat": "api-extractor run --verbose",
"test:mocha": "nyc mocha tests"
},
"mocha": {
"checkLeaks": true,
"exit": true
},
"files": [
"_ts3.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { StreamingRequest } from '../streamingRequest';
* Example possible implementations include WebSocket transport server or NamedPipe transport server.
*/
export interface IStreamingTransportServer {
start(): Promise<string>;
start(onListen?: () => void): Promise<string>;
disconnect(): void;
send(request: StreamingRequest): Promise<IReceiveResponse>;
isConnected?: boolean;
Expand Down
Loading

0 comments on commit 0ee95d2

Please sign in to comment.