Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assign event listener to socket close event on open before attempting post-open logic #1186

Merged
merged 7 commits into from
Jan 28, 2020
23 changes: 11 additions & 12 deletions src/common/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,18 +489,6 @@ export class Connection extends EventEmitter {
this._ws.on('error', error =>
this.emit('error', 'websocket', error.message, error)
)
// Finalize the connection and resolve all awaiting connect() requests
try {
this._retryConnectionBackoff.reset()
await this._subscribeToLedger()
this._startHeartbeatInterval()
this._connectionManager.resolveAllAwaiting()
this.emit('connected')
} catch (error) {
this._connectionManager.rejectAllAwaiting(error)
this.disconnect()
return
}
// Handle a closed connection: reconnect if it was unexpected
this._ws.once('close', code => {
this._clearHeartbeatInterval()
Expand All @@ -524,6 +512,17 @@ export class Connection extends EventEmitter {
}, retryTimeout)
}
})
// Finalize the connection and resolve all awaiting connect() requests
try {
this._retryConnectionBackoff.reset()
await this._subscribeToLedger()
this._startHeartbeatInterval()
this._connectionManager.resolveAllAwaiting()
this.emit('connected')
} catch (error) {
this._connectionManager.rejectAllAwaiting(error)
await this.disconnect().catch(() => {}) // Ignore this error, propagate the root cause.
}
})
return this._connectionManager.awaitConnection()
}
Expand Down
20 changes: 20 additions & 0 deletions test/connection-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,26 @@ describe('Connection', function() {
}
)

it('should clean up websocket connection if error after websocket is opened', async function() {
await this.api.disconnect();
// fail on connection
this.api.connection._subscribeToLedger = async () => {
throw new Error('error on _subscribeToLedger')
}
try {
await this.api.connect();
nickewansmith marked this conversation as resolved.
Show resolved Hide resolved
} catch (err) {
// _ws.close event listener should have cleaned up the socket when disconnect _ws.close is run on connection error
// do not fail on connection anymore
this.api.connection._subscribeToLedger = async () => {}
await new Promise((resolve, reject) => {
setTimeout(async () => {
nickewansmith marked this conversation as resolved.
Show resolved Hide resolved
this.api.connect().then(resolve).catch(reject); // should succeed and not throw websocket not cleaned up error
}, 1000)
})
}
})

it('should try to reconnect on empty subscribe response on reconnect', function(done) {
this.timeout(23000)
this.api.on('error', error => {
Expand Down