diff --git a/src/client/requestManager.ts b/src/client/requestManager.ts index 6cb0160f..86d9c097 100644 --- a/src/client/requestManager.ts +++ b/src/client/requestManager.ts @@ -1,4 +1,9 @@ -import { ResponseFormatError, RippledError, TimeoutError } from '../errors' +import { + ResponseFormatError, + RippledError, + TimeoutError, + XrplError, +} from '../errors' import { Response } from '../models/methods' import { BaseRequest, ErrorResponse } from '../models/methods/baseMethod' @@ -77,6 +82,7 @@ export default class RequestManager { public rejectAll(error: Error): void { this.promisesAwaitingResponse.forEach((_promise, id, _map) => { this.reject(id, error) + this.deletePromise(id) }) } @@ -88,13 +94,19 @@ export default class RequestManager { * @param request - Request to create. * @param timeout - Timeout length to catch hung responses. * @returns Request ID, new request form, and the promise for resolving the request. + * @throws XrplError if request with the same ID is already pending. */ public createRequest( request: T, timeout: number, ): [string | number, string, Promise] { - const newId = request.id ? request.id : this.nextId - this.nextId += 1 + let newId: string | number + if (request.id == null) { + newId = this.nextId + this.nextId += 1 + } else { + newId = request.id + } const newRequest = JSON.stringify({ ...request, id: newId }) const timer = setTimeout( () => this.reject(newId, new TimeoutError()), @@ -106,6 +118,9 @@ export default class RequestManager { if (timer.unref) { timer.unref() } + if (this.promisesAwaitingResponse.has(newId)) { + throw new XrplError(`Response with id '${newId}' is already pending`) + } const newPromise = new Promise( (resolve: (value: Response | PromiseLike) => void, reject) => { this.promisesAwaitingResponse.set(newId, { resolve, reject, timer }) @@ -125,8 +140,7 @@ export default class RequestManager { public handleResponse(response: Partial): void { if ( response.id == null || - !Number.isInteger(response.id) || - response.id < 0 + !(typeof response.id === 'string' || typeof response.id === 'number') ) { throw new ResponseFormatError('valid id not found in response', response) } @@ -165,7 +179,6 @@ export default class RequestManager { * @param id - ID of the request. */ private deletePromise(id: string | number): void { - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete -- Needs to delete promise after request has been fulfilled. - delete this.promisesAwaitingResponse[id] + this.promisesAwaitingResponse.delete(id) } } diff --git a/test/client/submitSignedTransaction.ts b/test/client/submitSignedTransaction.ts index c77af642..c796befa 100644 --- a/test/client/submitSignedTransaction.ts +++ b/test/client/submitSignedTransaction.ts @@ -62,7 +62,7 @@ describe('client.submitSignedTransaction', function () { this.mockRippled.addResponse('submit', rippled.submit.success) - assertRejects( + await assertRejects( this.client.submitSignedTransaction(signedTx), ValidationError, 'Transaction must be signed', diff --git a/test/connection.ts b/test/connection.ts index f061737e..1fc61f48 100644 --- a/test/connection.ts +++ b/test/connection.ts @@ -17,7 +17,7 @@ import { Connection } from 'xrpl-local/client/connection' import rippled from './fixtures/rippled' import { setupClient, teardownClient } from './setupClient' -import { ignoreWebSocketDisconnect } from './testUtils' +import { assertRejects, ignoreWebSocketDisconnect } from './testUtils' // how long before each test case times out const TIMEOUT = 20000 @@ -514,13 +514,13 @@ describe('Connection', function () { this.client.on('error', (errorCode, errorMessage, message) => { assert.strictEqual(errorCode, 'badMessage') assert.strictEqual(errorMessage, 'valid id not found in response') - assert.strictEqual(message, '{"type":"response","id":"must be integer"}') + assert.strictEqual(message, '{"type":"response","id":{}}') done() }) this.client.connection.onMessage( JSON.stringify({ type: 'response', - id: 'must be integer', + id: {}, }), ) }) @@ -619,4 +619,20 @@ describe('Connection', function () { .then(() => new Error('Should not have succeeded')) .catch(done()) }) + + it('should throw error if pending response with same ID', async function () { + const promise1 = this.client.connection.request({ + id: 'test', + command: 'ping', + }) + const promise2 = this.client.connection.request({ + id: 'test', + command: 'ping', + }) + await assertRejects( + Promise.all([promise1, promise2]), + XrplError, + "Response with id 'test' is already pending", + ) + }) }) diff --git a/test/mockRippled.ts b/test/mockRippled.ts index b1cb55c3..866bf45e 100644 --- a/test/mockRippled.ts +++ b/test/mockRippled.ts @@ -180,6 +180,16 @@ export default function createMockRippled(port: number): MockedWebSocketServer { ) } else if (request.data.closeServer) { conn.close() + } else if (request.data.delayedResponseIn) { + setTimeout(() => { + conn.send( + createResponse(request, { + status: 'success', + type: 'response', + result: {}, + }), + ) + }, request.data.delayedResponseIn) } } diff --git a/test/testUtils.ts b/test/testUtils.ts index b0c68f1c..224b56f4 100644 --- a/test/testUtils.ts +++ b/test/testUtils.ts @@ -58,7 +58,7 @@ export function assertResultMatch( * @param message - Expected error message/substring of the error message. */ export async function assertRejects( - promise: PromiseLike>, + promise: PromiseLike, instanceOf: any, message?: string | RegExp, ): Promise { @@ -72,7 +72,7 @@ export async function assertRejects( assert(error instanceof instanceOf, error.message) if (typeof message === 'string') { - assert.strictEqual(error.message, message) + assert.strictEqual(error.message, message, 'Messages do not match') } else if (message instanceof RegExp) { assert(message.test(error.message)) }