Skip to content

Commit 38ceecf

Browse files
committed
Merge pull request #371 from uProxy/trevj-retry
retry for SSH connections, with a re-useable library
2 parents 2a664a7 + 489e09c commit 38ceecf

File tree

4 files changed

+107
-27
lines changed

4 files changed

+107
-27
lines changed

src/churn-pipe/churn-pipe.ts

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,14 @@ import ipaddr = require('ipaddr.js');
1212
import logging = require('../logging/logging');
1313
import net = require('../net/net.types');
1414
import PassThrough = require('../transformers/passthrough');
15+
import promises = require('../promises/promises');
1516
import protean = require('../transformers/protean');
1617
import sequence = require('../transformers/byteSequenceShaper');
1718

1819
import Socket = freedom.UdpSocket.Socket;
1920

2021
var log :logging.Log = new logging.Log('churn-pipe');
2122

22-
// Retry an async function with exponential backoff for up to 2 seconds
23-
// before failing.
24-
var retry_ = <T>(func:() => Promise<T>, delayMs?:number) : Promise<T> => {
25-
delayMs = delayMs || 10;
26-
return func().catch((err) => {
27-
delayMs *= 2;
28-
if (delayMs > 2000) {
29-
return Promise.reject(err);
30-
}
31-
return new Promise<T>((F, R) => {
32-
setTimeout(() => {
33-
retry_(func, delayMs).then(F, R);
34-
}, delayMs);
35-
});
36-
});
37-
}
38-
3923
// Maps transformer names to class constructors.
4024
var transformers :{[name:string] : new() => Transformer} = {
4125
'caesar': caesar.CaesarCipher,
@@ -47,6 +31,10 @@ var transformers :{[name:string] : new() => Transformer} = {
4731
'sequenceShaper': sequence.ByteSequenceShaper
4832
};
4933

34+
// Local socket rebinding retry timing (see bindLocal)
35+
const INITIAL_REBIND_INTERVAL_MS = 10;
36+
const MAX_REBIND_INTERVAL_MS = 2000;
37+
5038
interface MirrorSet {
5139
// If true, these mirrors represent a remote endpoint that has been
5240
// explicitly signaled to us.
@@ -198,13 +186,13 @@ class Pipe {
198186
// This retry is needed because the browser releases the UDP port
199187
// asynchronously after we call close() on the RTCPeerConnection, so
200188
// this call to bind() may initially fail, until the port is released.
201-
portPromise = retry_(() => {
189+
portPromise = promises.retryWithExponentialBackoff(() => {
202190
log.debug('%1: trying to bind public endpoint: %2',
203191
this.name_, publicEndpoint);
204192
// TODO: Once https://github.com/freedomjs/freedom/issues/283 is
205193
// fixed, catch here, and only retry on an ALREADY_BOUND error.
206194
return socket.bind(anyInterface, publicEndpoint.port);
207-
}).then(() => {
195+
}, MAX_REBIND_INTERVAL_MS, INITIAL_REBIND_INTERVAL_MS).then(() => {
208196
log.debug('%1: successfully bound public endpoint: %2',
209197
this.name_, publicEndpoint);
210198
socket.on('onData', (recvFromInfo:freedom.UdpSocket.RecvFromInfo) => {

src/cloud/social/provider.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import arraybuffers = require('../../arraybuffers/arraybuffers');
77
import linefeeder = require('../../net/linefeeder');
88
import logging = require('../../logging/logging');
9+
import promises = require('../../promises/promises');
910
import queue = require('../../handler/queue');
1011

1112
// https://github.com/borisyankov/DefinitelyTyped/blob/master/ssh2/ssh2-tests.ts
@@ -25,6 +26,9 @@ const STORAGE_KEY = 'cloud-social-contacts';
2526
// Timeout for establishing an SSH connection.
2627
const CONNECT_TIMEOUT_MS = 10000;
2728

29+
// Maximum number of times to try establishing an SSH connection.
30+
const MAX_CONNECT_ATTEMPTS = 3;
31+
2832
// Credentials for accessing a cloud instance.
2933
// The serialised, base64 form is distributed amongst users.
3034
interface Invite {
@@ -199,15 +203,23 @@ export class CloudSocialProvider {
199203
});
200204
}
201205

202-
var connection = new Connection(invite, (message: Object) => {
203-
this.dispatchEvent_('onMessage', {
204-
from: makeClientState(invite.host),
205-
// SIGNAL_FROM_SERVER_PEER,
206-
message: JSON.stringify(makeVersionedPeerMessage(3002, message))
206+
let numAttempts = 0;
207+
const connect = () => {
208+
log.debug('connection attempt %1...', (++numAttempts));
209+
const connection = new Connection(invite, (message: Object) => {
210+
this.dispatchEvent_('onMessage', {
211+
from: makeClientState(invite.host),
212+
// SIGNAL_FROM_SERVER_PEER,
213+
message: JSON.stringify(makeVersionedPeerMessage(3002, message))
214+
});
207215
});
208-
});
209-
210-
this.clients_[invite.host] = connection.connect().then(() => {
216+
return connection.connect().then(() => {
217+
return connection;
218+
});
219+
};
220+
221+
this.clients_[invite.host] = promises.retry(connect,
222+
MAX_CONNECT_ATTEMPTS).then((connection:Connection) => {
211223
log.info('connected to zork on %1', invite.host);
212224

213225
// Fetch the banner, if available, then emit an instance message.

src/promises/promises.spec.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/// <reference path='../../../third_party/typings/es6-promise/es6-promise.d.ts' />
2+
/// <reference path='../../../third_party/typings/jasmine/jasmine.d.ts' />
3+
4+
import promises = require('./promises');
5+
6+
describe('retry', () => {
7+
const MAX_ATTEMPTS = 5;
8+
const RETURN_VALUE = 'hello';
9+
const REJECT_MESSAGE = 'goodbye';
10+
11+
it('only calls successful function once', (done) => {
12+
const f = jasmine.createSpy('spy');
13+
f.and.returnValue(Promise.resolve(RETURN_VALUE));
14+
promises.retry(f, MAX_ATTEMPTS).then((result: string) => {
15+
expect(f.calls.count()).toEqual(1);
16+
expect(result).toEqual(RETURN_VALUE);
17+
done();
18+
});
19+
});
20+
21+
it('calls multiple times until success', (done) => {
22+
const NUM_CALLS_BEFORE_SUCCESS = 3;
23+
let callCount = 0;
24+
const f = jasmine.createSpy('spy');
25+
f.and.callFake(() => {
26+
callCount++;
27+
if (callCount === NUM_CALLS_BEFORE_SUCCESS) {
28+
return Promise.resolve(RETURN_VALUE);
29+
} else {
30+
return Promise.reject('error');
31+
}
32+
});
33+
promises.retry(f, NUM_CALLS_BEFORE_SUCCESS + 1).then((result: string) => {
34+
expect(f.calls.count()).toEqual(NUM_CALLS_BEFORE_SUCCESS);
35+
expect(result).toEqual(RETURN_VALUE);
36+
done();
37+
});
38+
});
39+
40+
it('stops calling after the max number of failures', (done) => {
41+
const f = jasmine.createSpy('spy');
42+
f.and.returnValue(Promise.reject(new Error(REJECT_MESSAGE)));
43+
promises.retry(f, MAX_ATTEMPTS).catch((e: Error) => {
44+
expect(f.calls.count()).toEqual(MAX_ATTEMPTS);
45+
expect(e.message).toEqual(REJECT_MESSAGE);
46+
done();
47+
});
48+
});
49+
});

src/promises/promises.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/// <reference path='../../../third_party/typings/es6-promise/es6-promise.d.ts' />
2+
3+
// Invokes f up to maxAttempts number of times, resolving with its result
4+
// on the first success and rejecting on maxAttempts-th failure.
5+
export const retry = <T>(f: () => Promise<T>, maxAttempts: number): Promise<T> => {
6+
return f().catch((e:Error) => {
7+
--maxAttempts;
8+
if (maxAttempts > 0) {
9+
return retry(f, maxAttempts);
10+
} else {
11+
return Promise.reject(e);
12+
}
13+
});
14+
};
15+
16+
// Invokes f with exponential backoff between retries, resolving with its result
17+
// on the first success and rejecting on maxAttempts-th failure.
18+
export const retryWithExponentialBackoff = <T>(f: () => Promise<T>,
19+
maxIntervalMs: number, initialIntervalMs: number): Promise<T> => {
20+
return f().catch((e: Error) => {
21+
initialIntervalMs *= 2;
22+
if (initialIntervalMs > maxIntervalMs) {
23+
return Promise.reject(e);
24+
}
25+
return new Promise<T>((F, R) => {
26+
setTimeout(() => {
27+
retryWithExponentialBackoff(f, maxIntervalMs, initialIntervalMs).then(F, R);
28+
}, initialIntervalMs);
29+
});
30+
});
31+
};

0 commit comments

Comments
 (0)