From ec00fb2fb74d4da5ffb01236a8bd8deee5519ffd Mon Sep 17 00:00:00 2001 From: MantisClone Date: Fri, 5 Jan 2024 15:47:35 -0500 Subject: [PATCH 01/24] Move getConfirmedTransaction logic to new public async function * Update error message to recommend polling getConfirmedTransaction instead of getTransactionsByChannelId() --- .../request-client.js/src/http-data-access.ts | 51 +++++++++++-------- .../test/http-data-access.test.ts | 15 +++--- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index 429c73b45..89b00d0ec 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -100,8 +100,6 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { this.axiosConfig, ); - const transactionHash: string = normalizeKeccak256Hash(transactionData).value; - // Create the return result with EventEmitter const result: DataAccessTypes.IReturnPersistTransaction = Object.assign( new EventEmitter(), @@ -111,18 +109,7 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { // Try to get the confirmation new Promise((r) => setTimeout(r, this.httpConfig.getConfirmationDeferDelay)) .then(async () => { - const confirmedData = await this.fetchAndRetry( - '/getConfirmedTransaction', - { - transactionHash, - }, - { - maxRetries: this.httpConfig.getConfirmationMaxRetry, - retryDelay: this.httpConfig.getConfirmationRetryDelay, - exponentialBackoffDelay: this.httpConfig.getConfirmationExponentialBackoffDelay, - maxExponentialBackoffDelay: this.httpConfig.getConfirmationMaxExponentialBackoffDelay, - }, - ); + const confirmedData = await this.getConfirmedTransaction(transactionData); // when found, emit the event 'confirmed' result.emit('confirmed', confirmedData); }) @@ -130,13 +117,12 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { let error: Error = e; if (e.response.status === 404) { error = new Error( - `Transaction confirmation not received. Try polling - getTransactionsByChannelId() until the transaction is confirmed. - deferDelay: ${this.httpConfig.getConfirmationDeferDelay}ms, - maxRetries: ${this.httpConfig.getConfirmationMaxRetry}, - retryDelay: ${this.httpConfig.getConfirmationRetryDelay}ms, - exponentialBackoffDelay: ${this.httpConfig.getConfirmationExponentialBackoffDelay}ms, - maxExponentialBackoffDelay: ${this.httpConfig.getConfirmationMaxExponentialBackoffDelay}ms`, + `After calling /persistTransaction, polling /getConfirmedTransaction timed out. Try polling getConfirmedTransaction() until the transaction is confirmed. To avoid timeouts in the future, try adjusting the httpConfig values: + getConfirmationDeferDelay: ${this.httpConfig.getConfirmationDeferDelay}ms, + getConfirmationMaxRetries: ${this.httpConfig.getConfirmationMaxRetry}, + getConfirmationRetryDelay: ${this.httpConfig.getConfirmationRetryDelay}ms, + getConfirmationExponentialBackoffDelay: ${this.httpConfig.getConfirmationExponentialBackoffDelay}ms, + getConfirmationMaxExponentialBackoffDelay: ${this.httpConfig.getConfirmationMaxExponentialBackoffDelay}ms`, ); } result.emit('error', error); @@ -145,6 +131,29 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { return result; } + /** + * Gets a transaction from the node through HTTP. + * @param transactionData The transaction data + */ + public async getConfirmedTransaction( + transactionData: DataAccessTypes.ITransaction, + ): Promise { + const transactionHash: string = normalizeKeccak256Hash(transactionData).value; + + return await this.fetchAndRetry( + '/getConfirmedTransaction', + { + transactionHash, + }, + { + maxRetries: this.httpConfig.getConfirmationMaxRetry, + retryDelay: this.httpConfig.getConfirmationRetryDelay, + exponentialBackoffDelay: this.httpConfig.getConfirmationExponentialBackoffDelay, + maxExponentialBackoffDelay: this.httpConfig.getConfirmationMaxExponentialBackoffDelay, + }, + ); + } + /** * Gets the transactions for a channel from the node through HTTP. * diff --git a/packages/request-client.js/test/http-data-access.test.ts b/packages/request-client.js/test/http-data-access.test.ts index f29929f9b..50ee203b2 100644 --- a/packages/request-client.js/test/http-data-access.test.ts +++ b/packages/request-client.js/test/http-data-access.test.ts @@ -25,13 +25,14 @@ describe('HttpDataAccess', () => { }); void httpDataAccess.persistTransaction({}, '', []).then((returnPersistTransaction) => { returnPersistTransaction.on('error', (e: any) => { - expect(e.message).toBe(`Transaction confirmation not received. Try polling - getTransactionsByChannelId() until the transaction is confirmed. - deferDelay: 0ms, - maxRetries: 0, - retryDelay: 1000ms, - exponentialBackoffDelay: 0ms, - maxExponentialBackoffDelay: 30000ms`); + expect(e.message).toBe( + `After calling /persistTransaction, polling /getConfirmedTransaction timed out. Try calling getConfirmedTransaction() until the transaction is confirmed. To avoid timeouts in the future, try adjusting the httpConfig values: + getConfirmationDeferDelay: 0ms, + getConfirmationMaxRetries: 0, + getConfirmationRetryDelay: 1000ms, + getConfirmationExponentialBackoffDelay: 0ms, + getConfirmationMaxExponentialBackoffDelay: 30000ms`, + ); done(); }); }); From c95aef6f95c7701c5980ee91fcb749b1925d0a46 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Fri, 5 Jan 2024 15:49:11 -0500 Subject: [PATCH 02/24] Adjust retry logic to make exponentialBackoffDelay more intuitive * Previously, 500 meant retry after 1, 2, 4, 8... seconds. * Now, 1000 means retry after 1, 2, 4, 8... seconds. --- packages/utils/src/retry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/src/retry.ts b/packages/utils/src/retry.ts index 3521defa8..3b32e0747 100644 --- a/packages/utils/src/retry.ts +++ b/packages/utils/src/retry.ts @@ -61,7 +61,7 @@ const retry = ( setTimeout( resolve, retryDelay + - Math.min(maxExponentialBackoffDelay, exponentialBackoffDelay * 2 ** retry), + Math.min(maxExponentialBackoffDelay, (exponentialBackoffDelay / 2) * 2 ** retry), ), ); From be2e95f2c3209e5becc8e9f902ee6875fc7031af Mon Sep 17 00:00:00 2001 From: MantisClone Date: Fri, 5 Jan 2024 16:13:45 -0500 Subject: [PATCH 03/24] Update exponential backoff test --- packages/utils/test/retry.test.ts | 37 +++++++++++++++++++------------ 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index a3110edf5..ff791b8dc 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -132,37 +132,46 @@ describe('Retry', () => { }); retry(throwFn, { - exponentialBackoffDelay: 1000, - maxExponentialBackoffDelay: 7000, + maxRetries: 5, + retryDelay: 0, + exponentialBackoffDelay: 1000, // 1s + maxExponentialBackoffDelay: 7000, // 7s })(); // Should call immediately expect(throwFn).toHaveBeenCalledTimes(1); - // Exponential backoff should only call a second time after 2000ms - jest.advanceTimersByTime(1100); + // Call 2nd time after 1s + jest.advanceTimersByTime(999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(1); - jest.advanceTimersByTime(1100); + jest.advanceTimersByTime(1000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(2); - // Exponential backoff should call a third time after 4100ms - jest.advanceTimersByTime(4100); + // Call 3rd time after 2s + jest.advanceTimersByTime(1999); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(2); + jest.advanceTimersByTime(2000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(3); - // Exponential backoff should call a fourth time after 7100ms - // since maxExponentialBackoffDelay (7000) < 8000 - jest.advanceTimersByTime(7100); + // Call 4th time after 4s + jest.advanceTimersByTime(3999); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(3); + jest.advanceTimersByTime(4000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); - // Exponential backoff should call a fifth time after 7100ms - // since maxExponentialBackoffDelay (7000) < 8000 - jest.advanceTimersByTime(7100); + // Don't call 5th time after 8s because 8s > maxExponentialBackoffDelay + jest.advanceTimersByTime(7999); await Promise.resolve(); - expect(throwFn).toHaveBeenCalledTimes(5); + expect(throwFn).toHaveBeenCalledTimes(4); + jest.advanceTimersByTime(8000); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(4); jest.useRealTimers(); }); From 29d0aaacb23719d6d9b8decd7af9e8699f67e211 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Fri, 5 Jan 2024 16:30:18 -0500 Subject: [PATCH 04/24] Update exponential backoff test --- packages/utils/test/retry.test.ts | 32 +++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index ff791b8dc..1dbf3bd22 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -132,10 +132,10 @@ describe('Retry', () => { }); retry(throwFn, { - maxRetries: 5, + maxRetries: 30, retryDelay: 0, exponentialBackoffDelay: 1000, // 1s - maxExponentialBackoffDelay: 7000, // 7s + maxExponentialBackoffDelay: 30000, // 30s })(); // Should call immediately @@ -165,13 +165,37 @@ describe('Retry', () => { await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); - // Don't call 5th time after 8s because 8s > maxExponentialBackoffDelay + // Call 5th time after 8s jest.advanceTimersByTime(7999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); jest.advanceTimersByTime(8000); await Promise.resolve(); - expect(throwFn).toHaveBeenCalledTimes(4); + expect(throwFn).toHaveBeenCalledTimes(5); + + // Call 6th time after 16s + jest.advanceTimersByTime(15999); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(5); + jest.advanceTimersByTime(16000); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(6); + + // Call 7th time after 32s + jest.advanceTimersByTime(31999); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(6); + jest.advanceTimersByTime(32000); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(7); + + // Don't call 8th time, even after 64s + jest.advanceTimersByTime(63999); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(7); + jest.advanceTimersByTime(64000); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(7); jest.useRealTimers(); }); From bc405b39796970afe1200fd467857e496b701d86 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 16:59:17 -0400 Subject: [PATCH 05/24] Update retry test --- packages/utils/test/retry.test.ts | 54 ++++++++++++++++++------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index 1dbf3bd22..9e7be8894 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -133,69 +133,77 @@ describe('Retry', () => { retry(throwFn, { maxRetries: 30, - retryDelay: 0, + retryDelay: 2000, // 2s exponentialBackoffDelay: 1000, // 1s - maxExponentialBackoffDelay: 30000, // 30s + maxExponentialBackoffDelay: 122000, // 122s })(); // Should call immediately expect(throwFn).toHaveBeenCalledTimes(1); - // Call 2nd time after 1s - jest.advanceTimersByTime(999); + // Call 2nd time after 3s + jest.advanceTimersByTime(2999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(1); - jest.advanceTimersByTime(1000); + jest.advanceTimersByTime(3000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(2); - // Call 3rd time after 2s - jest.advanceTimersByTime(1999); + // Call 3rd time after 4s + jest.advanceTimersByTime(3999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(2); - jest.advanceTimersByTime(2000); + jest.advanceTimersByTime(4000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(3); // Call 4th time after 4s - jest.advanceTimersByTime(3999); + jest.advanceTimersByTime(5999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(3); - jest.advanceTimersByTime(4000); + jest.advanceTimersByTime(6000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); - // Call 5th time after 8s - jest.advanceTimersByTime(7999); + // Call 5th time after 10s + jest.advanceTimersByTime(9999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); - jest.advanceTimersByTime(8000); + jest.advanceTimersByTime(10000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(5); - // Call 6th time after 16s - jest.advanceTimersByTime(15999); + // Call 6th time after 18s + jest.advanceTimersByTime(17999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(5); - jest.advanceTimersByTime(16000); + jest.advanceTimersByTime(18000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(6); - // Call 7th time after 32s - jest.advanceTimersByTime(31999); + // Call 7th time after 34s + jest.advanceTimersByTime(33999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(6); - jest.advanceTimersByTime(32000); + jest.advanceTimersByTime(34000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(7); - // Don't call 8th time, even after 64s - jest.advanceTimersByTime(63999); + // Call 8th time after 66s + jest.advanceTimersByTime(65999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(7); - jest.advanceTimersByTime(64000); + jest.advanceTimersByTime(66000); await Promise.resolve(); - expect(throwFn).toHaveBeenCalledTimes(7); + expect(throwFn).toHaveBeenCalledTimes(8); + + // Call 9th time after 122s + jest.advanceTimersByTime(121999); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(8); + jest.advanceTimersByTime(122000); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(9); jest.useRealTimers(); }); From ca012983e315f4bc309f9789d34e35854a630103 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 21:24:13 -0400 Subject: [PATCH 06/24] Add note to README how to run tests for single package --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 6fb2bb97d..d6c8c2178 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,12 @@ Test all the packages in the monorepo. yarn run test ``` +Test a specific package. + +```bash +yarn workspace @requestnetwork/package-name test +``` + ## License [MIT](https://github.com/RequestNetwork/requestNetwork/blob/master/LICENSE) From 077a91f2cd9871aba5847ae8c5c99fb93be6216f Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 21:24:27 -0400 Subject: [PATCH 07/24] Update error message to be more descriptive --- packages/request-client.js/src/http-data-access.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index 04f3ba0fa..f7fd446e0 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -115,12 +115,7 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { let error: Error = e; if (e.status === 404) { error = new Error( - `After calling /persistTransaction, polling /getConfirmedTransaction timed out. Try polling getConfirmedTransaction() until the transaction is confirmed. To avoid timeouts in the future, try adjusting the httpConfig values: - getConfirmationDeferDelay: ${this.httpConfig.getConfirmationDeferDelay}ms, - getConfirmationMaxRetries: ${this.httpConfig.getConfirmationMaxRetry}, - getConfirmationRetryDelay: ${this.httpConfig.getConfirmationRetryDelay}ms, - getConfirmationExponentialBackoffDelay: ${this.httpConfig.getConfirmationExponentialBackoffDelay}ms, - getConfirmationMaxExponentialBackoffDelay: ${this.httpConfig.getConfirmationMaxExponentialBackoffDelay}ms`, + `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object: getConfirmationDeferDelay: ${this.httpConfig.getConfirmationDeferDelay}ms, getConfirmationMaxRetries: ${this.httpConfig.getConfirmationMaxRetry}, getConfirmationRetryDelay: ${this.httpConfig.getConfirmationRetryDelay}ms, getConfirmationExponentialBackoffDelay: ${this.httpConfig.getConfirmationExponentialBackoffDelay}ms, getConfirmationMaxExponentialBackoffDelay: ${this.httpConfig.getConfirmationMaxExponentialBackoffDelay}ms`, ); } result.emit('error', error); From 9ca0e9691cebf0183de7902532c9f40071383ff1 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 21:26:28 -0400 Subject: [PATCH 08/24] Check that total elapsed time is as expected --- packages/utils/test/retry.test.ts | 70 +++++++++++++++++++------------ 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index 9e7be8894..593c5b898 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -133,78 +133,96 @@ describe('Retry', () => { retry(throwFn, { maxRetries: 30, - retryDelay: 2000, // 2s + retryDelay: 0, exponentialBackoffDelay: 1000, // 1s maxExponentialBackoffDelay: 122000, // 122s })(); - // Should call immediately + // Should call immediately (0ms total elapsed) expect(throwFn).toHaveBeenCalledTimes(1); - // Call 2nd time after 3s - jest.advanceTimersByTime(2999); + expect(jest.getRealSystemTime()).toBe(0); + + // Call 2nd time after 1s (1000ms total elapsed) + jest.advanceTimersByTime(999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(1); - jest.advanceTimersByTime(3000); + jest.advanceTimersByTime(1000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(2); - // Call 3rd time after 4s - jest.advanceTimersByTime(3999); + expect(jest.getRealSystemTime()).toBe(1000); + + // Call 3rd time after 2s (3000ms total elapsed) + jest.advanceTimersByTime(1999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(2); - jest.advanceTimersByTime(4000); + jest.advanceTimersByTime(2000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(3); - // Call 4th time after 4s - jest.advanceTimersByTime(5999); + expect(jest.getRealSystemTime()).toBe(3000); + + // Call 4th time after 4s (7000ms total elapsed) + jest.advanceTimersByTime(3999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(3); - jest.advanceTimersByTime(6000); + jest.advanceTimersByTime(4000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); - // Call 5th time after 10s - jest.advanceTimersByTime(9999); + expect(jest.getRealSystemTime()).toBe(7000); + + // Call 5th time after 8s (15000ms total elapsed) + jest.advanceTimersByTime(7999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); - jest.advanceTimersByTime(10000); + jest.advanceTimersByTime(8000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(5); - // Call 6th time after 18s - jest.advanceTimersByTime(17999); + expect(jest.getRealSystemTime()).toBe(15000); + + // Call 6th time after 16s (31000ms total elapsed) + jest.advanceTimersByTime(15999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(5); - jest.advanceTimersByTime(18000); + jest.advanceTimersByTime(16000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(6); - // Call 7th time after 34s - jest.advanceTimersByTime(33999); + expect(jest.getRealSystemTime()).toBe(31000); + + // Call 7th time after 32s (63000ms total elapsed) + jest.advanceTimersByTime(31999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(6); - jest.advanceTimersByTime(34000); + jest.advanceTimersByTime(32000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(7); - // Call 8th time after 66s - jest.advanceTimersByTime(65999); + expect(jest.getRealSystemTime()).toBe(63000); + + // Call 8th time after 64s (127000ms total elapsed) + jest.advanceTimersByTime(63999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(7); - jest.advanceTimersByTime(66000); + jest.advanceTimersByTime(64000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(8); - // Call 9th time after 122s - jest.advanceTimersByTime(121999); + expect(jest.getRealSystemTime()).toBe(127000); + + // Call 9th time after 120s (247000ms total elapsed) + jest.advanceTimersByTime(119999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(8); - jest.advanceTimersByTime(122000); + jest.advanceTimersByTime(120000); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(9); + expect(jest.getRealSystemTime()).toBe(247000); + jest.useRealTimers(); }); }); From 3b7801c0d9f8a43356e4b6708c52d35d0617c429 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 21:28:17 -0400 Subject: [PATCH 09/24] Update test to match new error message --- packages/request-client.js/test/http-data-access.test.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/request-client.js/test/http-data-access.test.ts b/packages/request-client.js/test/http-data-access.test.ts index de37d4dcb..0589b33ee 100644 --- a/packages/request-client.js/test/http-data-access.test.ts +++ b/packages/request-client.js/test/http-data-access.test.ts @@ -32,12 +32,7 @@ describe('HttpDataAccess', () => { void httpDataAccess.persistTransaction({}, '', []).then((returnPersistTransaction) => { returnPersistTransaction.on('error', (e: any) => { expect(e.message).toBe( - `After calling /persistTransaction, polling /getConfirmedTransaction timed out. Try calling getConfirmedTransaction() until the transaction is confirmed. To avoid timeouts in the future, try adjusting the httpConfig values: - getConfirmationDeferDelay: 0ms, - getConfirmationMaxRetries: 0, - getConfirmationRetryDelay: 1000ms, - getConfirmationExponentialBackoffDelay: 0ms, - getConfirmationMaxExponentialBackoffDelay: 30000ms`, + `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object: getConfirmationDeferDelay: 0ms, getConfirmationMaxRetries: 0, getConfirmationRetryDelay: 1000ms, getConfirmationExponentialBackoffDelay: 0ms, getConfirmationMaxExponentialBackoffDelay: 30000ms`, ); done(); }); From 2a4a47b3a5518aa02caee79b4705a093ecebc02c Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 23:24:13 -0400 Subject: [PATCH 10/24] Fix test according to CodeRabbit recommendation --- packages/request-client.js/test/http-data-access.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/request-client.js/test/http-data-access.test.ts b/packages/request-client.js/test/http-data-access.test.ts index 0589b33ee..79ca56276 100644 --- a/packages/request-client.js/test/http-data-access.test.ts +++ b/packages/request-client.js/test/http-data-access.test.ts @@ -30,12 +30,18 @@ describe('HttpDataAccess', () => { }, }); void httpDataAccess.persistTransaction({}, '', []).then((returnPersistTransaction) => { + let errorEmitted = false; returnPersistTransaction.on('error', (e: any) => { + errorEmitted = true; expect(e.message).toBe( `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object: getConfirmationDeferDelay: 0ms, getConfirmationMaxRetries: 0, getConfirmationRetryDelay: 1000ms, getConfirmationExponentialBackoffDelay: 0ms, getConfirmationMaxExponentialBackoffDelay: 30000ms`, ); - done(); }); + + setTimeout(() => { + expect(errorEmitted).toBe(true); + done(); + }, 1000); }); }); }); From 497f6aa3c1bb7a15dd88a58fb56d672f099140f7 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 23:25:50 -0400 Subject: [PATCH 11/24] Make instruction more specific with example package name --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d6c8c2178..d53aa65b6 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,7 @@ yarn run test Test a specific package. ```bash -yarn workspace @requestnetwork/package-name test +yarn workspace @requestnetwork/request-client.js test ``` ## License From 3d4b407fe2d0ee9bff685969cd76dac5fcf392d4 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 23:42:50 -0400 Subject: [PATCH 12/24] Fixes based on CodeRabbit comments --- README.md | 2 +- .../request-client.js/src/http-data-access.ts | 2 +- packages/utils/test/retry.test.ts | 49 ++++++++----------- 3 files changed, 23 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index d53aa65b6..9546524a0 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ Test all the packages in the monorepo. yarn run test ``` -Test a specific package. +Test a specific package by replacing `@requestnetwork/request-client.js` with the desired package name: ```bash yarn workspace @requestnetwork/request-client.js test diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index f7fd446e0..f083bb3b1 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -115,7 +115,7 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { let error: Error = e; if (e.status === 404) { error = new Error( - `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object: getConfirmationDeferDelay: ${this.httpConfig.getConfirmationDeferDelay}ms, getConfirmationMaxRetries: ${this.httpConfig.getConfirmationMaxRetry}, getConfirmationRetryDelay: ${this.httpConfig.getConfirmationRetryDelay}ms, getConfirmationExponentialBackoffDelay: ${this.httpConfig.getConfirmationExponentialBackoffDelay}ms, getConfirmationMaxExponentialBackoffDelay: ${this.httpConfig.getConfirmationMaxExponentialBackoffDelay}ms`, + `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object: getConfirmationDeferDelay: ${this.httpConfig.getConfirmationDeferDelay}ms, getConfirmationMaxRetry: ${this.httpConfig.getConfirmationMaxRetry}, getConfirmationRetryDelay: ${this.httpConfig.getConfirmationRetryDelay}ms, getConfirmationExponentialBackoffDelay: ${this.httpConfig.getConfirmationExponentialBackoffDelay}ms, getConfirmationMaxExponentialBackoffDelay: ${this.httpConfig.getConfirmationMaxExponentialBackoffDelay}ms`, ); } result.emit('error', error); diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index 593c5b898..d19518672 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -134,94 +134,87 @@ describe('Retry', () => { retry(throwFn, { maxRetries: 30, retryDelay: 0, + // Exponential backoff starting at 1s, doubling each time, up to a maximum of 128s exponentialBackoffDelay: 1000, // 1s - maxExponentialBackoffDelay: 122000, // 122s + maxExponentialBackoffDelay: 128000, // 128s })(); // Should call immediately (0ms total elapsed) expect(throwFn).toHaveBeenCalledTimes(1); - expect(jest.getRealSystemTime()).toBe(0); + expect(Date.now()).toBe(0); // Call 2nd time after 1s (1000ms total elapsed) jest.advanceTimersByTime(999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(1); - jest.advanceTimersByTime(1000); + jest.advanceTimersByTime(1); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(2); + expect(Date.now()).toBe(1000); - expect(jest.getRealSystemTime()).toBe(1000); - - // Call 3rd time after 2s (3000ms total elapsed) + // Call 3rd time after 3s (3000ms total elapsed) jest.advanceTimersByTime(1999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(2); - jest.advanceTimersByTime(2000); + jest.advanceTimersByTime(1); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(3); - - expect(jest.getRealSystemTime()).toBe(3000); + expect(Date.now()).toBe(3000); // Call 4th time after 4s (7000ms total elapsed) jest.advanceTimersByTime(3999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(3); - jest.advanceTimersByTime(4000); + jest.advanceTimersByTime(1); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); - - expect(jest.getRealSystemTime()).toBe(7000); + expect(Date.now()).toBe(7000); // Call 5th time after 8s (15000ms total elapsed) jest.advanceTimersByTime(7999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); - jest.advanceTimersByTime(8000); + jest.advanceTimersByTime(1); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(5); - - expect(jest.getRealSystemTime()).toBe(15000); + expect(Date.now()).toBe(15000); // Call 6th time after 16s (31000ms total elapsed) jest.advanceTimersByTime(15999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(5); - jest.advanceTimersByTime(16000); + jest.advanceTimersByTime(1); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(6); - - expect(jest.getRealSystemTime()).toBe(31000); + expect(Date.now()).toBe(31000); // Call 7th time after 32s (63000ms total elapsed) jest.advanceTimersByTime(31999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(6); - jest.advanceTimersByTime(32000); + jest.advanceTimersByTime(1); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(7); - - expect(jest.getRealSystemTime()).toBe(63000); + expect(Date.now()).toBe(63000); // Call 8th time after 64s (127000ms total elapsed) jest.advanceTimersByTime(63999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(7); - jest.advanceTimersByTime(64000); + jest.advanceTimersByTime(1); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(8); + expect(Date.now()).toBe(127000); - expect(jest.getRealSystemTime()).toBe(127000); - - // Call 9th time after 120s (247000ms total elapsed) + // Call 9th time after 120s (255000 total elapsed) jest.advanceTimersByTime(119999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(8); - jest.advanceTimersByTime(120000); + jest.advanceTimersByTime(1); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(9); - - expect(jest.getRealSystemTime()).toBe(247000); + expect(Date.now()).toBe(255000); jest.useRealTimers(); }); From d6339e29e946f0a03bf59f2ee0162858007d6633 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 23:48:46 -0400 Subject: [PATCH 13/24] Fix test per CodeRabbit comment --- .../request-client.js/test/http-data-access.test.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/request-client.js/test/http-data-access.test.ts b/packages/request-client.js/test/http-data-access.test.ts index 79ca56276..1c5eabd2f 100644 --- a/packages/request-client.js/test/http-data-access.test.ts +++ b/packages/request-client.js/test/http-data-access.test.ts @@ -29,19 +29,14 @@ describe('HttpDataAccess', () => { getConfirmationMaxRetry: 0, }, }); - void httpDataAccess.persistTransaction({}, '', []).then((returnPersistTransaction) => { - let errorEmitted = false; + const returnPersistTransaction = await httpDataAccess.persistTransaction({}, '', []); + await new Promise((resolve) => { returnPersistTransaction.on('error', (e: any) => { - errorEmitted = true; expect(e.message).toBe( `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object: getConfirmationDeferDelay: 0ms, getConfirmationMaxRetries: 0, getConfirmationRetryDelay: 1000ms, getConfirmationExponentialBackoffDelay: 0ms, getConfirmationMaxExponentialBackoffDelay: 30000ms`, ); + resolve(); }); - - setTimeout(() => { - expect(errorEmitted).toBe(true); - done(); - }, 1000); }); }); }); From 1fa766b6d420945693d8c67ecb07fbee960a6b54 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 23:55:38 -0400 Subject: [PATCH 14/24] Fix test --- packages/utils/test/retry.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index d19518672..ebd7fa0ec 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -207,8 +207,8 @@ describe('Retry', () => { expect(throwFn).toHaveBeenCalledTimes(8); expect(Date.now()).toBe(127000); - // Call 9th time after 120s (255000 total elapsed) - jest.advanceTimersByTime(119999); + // Call 9th time after 128s (255000 total elapsed) + jest.advanceTimersByTime(127999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(8); jest.advanceTimersByTime(1); From 7f2a5bd948a6a1e7c4149687bbfdf51fbc1596d6 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Tue, 1 Oct 2024 23:59:47 -0400 Subject: [PATCH 15/24] update test --- packages/utils/test/retry.test.ts | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index ebd7fa0ec..426b80d9b 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -134,9 +134,9 @@ describe('Retry', () => { retry(throwFn, { maxRetries: 30, retryDelay: 0, - // Exponential backoff starting at 1s, doubling each time, up to a maximum of 128s + // Exponential backoff starting at 1s, doubling each time, up to a maximum of 64s, yielding a total timeout of 127s exponentialBackoffDelay: 1000, // 1s - maxExponentialBackoffDelay: 128000, // 128s + maxExponentialBackoffDelay: 64000, // 64s })(); // Should call immediately (0ms total elapsed) @@ -207,15 +207,6 @@ describe('Retry', () => { expect(throwFn).toHaveBeenCalledTimes(8); expect(Date.now()).toBe(127000); - // Call 9th time after 128s (255000 total elapsed) - jest.advanceTimersByTime(127999); - await Promise.resolve(); - expect(throwFn).toHaveBeenCalledTimes(8); - jest.advanceTimersByTime(1); - await Promise.resolve(); - expect(throwFn).toHaveBeenCalledTimes(9); - expect(Date.now()).toBe(255000); - jest.useRealTimers(); }); }); From e0c5e4bbbb5f3cbdc0124f46b5dfa86c20ec0be0 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Wed, 2 Oct 2024 00:35:06 -0400 Subject: [PATCH 16/24] Fix test --- packages/utils/test/retry.test.ts | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index 426b80d9b..28be228ee 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -132,19 +132,21 @@ describe('Retry', () => { }); retry(throwFn, { - maxRetries: 30, retryDelay: 0, - // Exponential backoff starting at 1s, doubling each time, up to a maximum of 64s, yielding a total timeout of 127s + // Exponential backoff starting at 1s, doubling each time, up to a maximum of 64s and max 8 retries, yielding a total timeout of 127s + maxRetries: 7, exponentialBackoffDelay: 1000, // 1s maxExponentialBackoffDelay: 64000, // 64s - })(); + })().catch(() => { + // Do nothing + }); - // Should call immediately (0ms total elapsed) + // Should call immediately (1 total calls, 0ms total elapsed) expect(throwFn).toHaveBeenCalledTimes(1); expect(Date.now()).toBe(0); - // Call 2nd time after 1s (1000ms total elapsed) + // 1st retry after 1s (2 total calls, 1000ms total elapsed) jest.advanceTimersByTime(999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(1); @@ -153,7 +155,7 @@ describe('Retry', () => { expect(throwFn).toHaveBeenCalledTimes(2); expect(Date.now()).toBe(1000); - // Call 3rd time after 3s (3000ms total elapsed) + // 2nd retry after 3s (3 total calls, 3000ms total elapsed) jest.advanceTimersByTime(1999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(2); @@ -162,7 +164,7 @@ describe('Retry', () => { expect(throwFn).toHaveBeenCalledTimes(3); expect(Date.now()).toBe(3000); - // Call 4th time after 4s (7000ms total elapsed) + // 3rd retry after 4s (4 total calls, 7000ms total elapsed) jest.advanceTimersByTime(3999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(3); @@ -171,7 +173,7 @@ describe('Retry', () => { expect(throwFn).toHaveBeenCalledTimes(4); expect(Date.now()).toBe(7000); - // Call 5th time after 8s (15000ms total elapsed) + // 4th retry after 8s (5 total calls, 15000ms total elapsed) jest.advanceTimersByTime(7999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(4); @@ -180,7 +182,7 @@ describe('Retry', () => { expect(throwFn).toHaveBeenCalledTimes(5); expect(Date.now()).toBe(15000); - // Call 6th time after 16s (31000ms total elapsed) + // 5th retry after 16s (6 total calls, 31000ms total elapsed) jest.advanceTimersByTime(15999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(5); @@ -189,7 +191,7 @@ describe('Retry', () => { expect(throwFn).toHaveBeenCalledTimes(6); expect(Date.now()).toBe(31000); - // Call 7th time after 32s (63000ms total elapsed) + // 6th retry after 32s (7 total calls, 63000ms total elapsed) jest.advanceTimersByTime(31999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(6); @@ -198,7 +200,7 @@ describe('Retry', () => { expect(throwFn).toHaveBeenCalledTimes(7); expect(Date.now()).toBe(63000); - // Call 8th time after 64s (127000ms total elapsed) + // 7th retry after 64s (8 total calls, 127000ms total elapsed) jest.advanceTimersByTime(63999); await Promise.resolve(); expect(throwFn).toHaveBeenCalledTimes(7); @@ -207,6 +209,12 @@ describe('Retry', () => { expect(throwFn).toHaveBeenCalledTimes(8); expect(Date.now()).toBe(127000); + // No further retries + jest.advanceTimersByTime(1000000000); + await Promise.resolve(); + expect(throwFn).toHaveBeenCalledTimes(8); + expect(Date.now()).toBe(1000127000); + jest.useRealTimers(); }); }); From 7b76550547dc70d432f06b657873b4d7333a7186 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Wed, 2 Oct 2024 00:41:22 -0400 Subject: [PATCH 17/24] Update defaults --- packages/request-client.js/src/http-config-defaults.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/request-client.js/src/http-config-defaults.ts b/packages/request-client.js/src/http-config-defaults.ts index 78edc2992..5d2909f5c 100644 --- a/packages/request-client.js/src/http-config-defaults.ts +++ b/packages/request-client.js/src/http-config-defaults.ts @@ -6,10 +6,12 @@ const config: ClientTypes.IHttpDataAccessConfig = { httpRequestRetryDelay: 100, httpRequestExponentialBackoffDelay: 0, httpRequestMaxExponentialBackoffDelay: 30000, - getConfirmationMaxRetry: 30, - getConfirmationRetryDelay: 1000, - getConfirmationExponentialBackoffDelay: 0, - getConfirmationMaxExponentialBackoffDelay: 30000, + + // Exponential backoff starting at 1s, doubling each time, up to a maximum of 64s and max 7 retries with an initial 3s defer delay, yielding a total of 8 calls and total timeout of 130s + getConfirmationMaxRetry: 7, + getConfirmationRetryDelay: 0, + getConfirmationExponentialBackoffDelay: 1000, + getConfirmationMaxExponentialBackoffDelay: 64000, getConfirmationDeferDelay: 3000, }; From a246c91a8827cc10a43e5a8219b2a23a69e4a731 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Wed, 2 Oct 2024 00:43:45 -0400 Subject: [PATCH 18/24] Update error message --- packages/request-client.js/src/http-data-access.ts | 2 +- packages/request-client.js/test/http-data-access.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index f083bb3b1..162811a03 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -115,7 +115,7 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { let error: Error = e; if (e.status === 404) { error = new Error( - `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object: getConfirmationDeferDelay: ${this.httpConfig.getConfirmationDeferDelay}ms, getConfirmationMaxRetry: ${this.httpConfig.getConfirmationMaxRetry}, getConfirmationRetryDelay: ${this.httpConfig.getConfirmationRetryDelay}ms, getConfirmationExponentialBackoffDelay: ${this.httpConfig.getConfirmationExponentialBackoffDelay}ms, getConfirmationMaxExponentialBackoffDelay: ${this.httpConfig.getConfirmationMaxExponentialBackoffDelay}ms`, + `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object. The current httpConfig settings are: getConfirmationDeferDelay: ${this.httpConfig.getConfirmationDeferDelay}ms, getConfirmationMaxRetry: ${this.httpConfig.getConfirmationMaxRetry}, getConfirmationRetryDelay: ${this.httpConfig.getConfirmationRetryDelay}ms, getConfirmationExponentialBackoffDelay: ${this.httpConfig.getConfirmationExponentialBackoffDelay}ms, getConfirmationMaxExponentialBackoffDelay: ${this.httpConfig.getConfirmationMaxExponentialBackoffDelay}ms`, ); } result.emit('error', error); diff --git a/packages/request-client.js/test/http-data-access.test.ts b/packages/request-client.js/test/http-data-access.test.ts index 1c5eabd2f..f17200d9b 100644 --- a/packages/request-client.js/test/http-data-access.test.ts +++ b/packages/request-client.js/test/http-data-access.test.ts @@ -33,7 +33,7 @@ describe('HttpDataAccess', () => { await new Promise((resolve) => { returnPersistTransaction.on('error', (e: any) => { expect(e.message).toBe( - `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object: getConfirmationDeferDelay: 0ms, getConfirmationMaxRetries: 0, getConfirmationRetryDelay: 1000ms, getConfirmationExponentialBackoffDelay: 0ms, getConfirmationMaxExponentialBackoffDelay: 30000ms`, + `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object. The current httpConfig settings are: getConfirmationDeferDelay: 0ms, getConfirmationMaxRetries: 0, getConfirmationRetryDelay: 1000ms, getConfirmationExponentialBackoffDelay: 0ms, getConfirmationMaxExponentialBackoffDelay: 30000ms`, ); resolve(); }); From b699fbd3418d06a65a6ca5b079f9db086a802609 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Wed, 2 Oct 2024 00:50:57 -0400 Subject: [PATCH 19/24] Clean up error messages --- packages/request-client.js/src/http-config-defaults.ts | 2 +- packages/utils/test/retry.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/request-client.js/src/http-config-defaults.ts b/packages/request-client.js/src/http-config-defaults.ts index 5d2909f5c..e5a5a436c 100644 --- a/packages/request-client.js/src/http-config-defaults.ts +++ b/packages/request-client.js/src/http-config-defaults.ts @@ -7,7 +7,7 @@ const config: ClientTypes.IHttpDataAccessConfig = { httpRequestExponentialBackoffDelay: 0, httpRequestMaxExponentialBackoffDelay: 30000, - // Exponential backoff starting at 1s, doubling each time, up to a maximum of 64s and max 7 retries with an initial 3s defer delay, yielding a total of 8 calls and total timeout of 130s + // Exponential backoff starting at 1s, doubling after each retry, up to a maximum of 64s and max 7 retries with an initial 3s defer delay, yielding a total of 8 calls and total timeout of 130s getConfirmationMaxRetry: 7, getConfirmationRetryDelay: 0, getConfirmationExponentialBackoffDelay: 1000, diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index 28be228ee..7a4bd94ff 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -133,7 +133,7 @@ describe('Retry', () => { retry(throwFn, { retryDelay: 0, - // Exponential backoff starting at 1s, doubling each time, up to a maximum of 64s and max 8 retries, yielding a total timeout of 127s + // Exponential backoff starting at 1s, doubling after each retry, up to a maximum of 64s and max 7 retries, yielding a total of 8 call snad total timeout of 127s maxRetries: 7, exponentialBackoffDelay: 1000, // 1s maxExponentialBackoffDelay: 64000, // 64s From 3647631719702eacccf37d8450d2b8575f91be99 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Wed, 2 Oct 2024 00:56:20 -0400 Subject: [PATCH 20/24] Add a timeout to the Promise to ensure the test doesn't hang indefinitely if the error event is never emitted. --- .../test/http-data-access.test.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/request-client.js/test/http-data-access.test.ts b/packages/request-client.js/test/http-data-access.test.ts index f17200d9b..7ed6b846a 100644 --- a/packages/request-client.js/test/http-data-access.test.ts +++ b/packages/request-client.js/test/http-data-access.test.ts @@ -30,14 +30,17 @@ describe('HttpDataAccess', () => { }, }); const returnPersistTransaction = await httpDataAccess.persistTransaction({}, '', []); - await new Promise((resolve) => { - returnPersistTransaction.on('error', (e: any) => { - expect(e.message).toBe( - `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object. The current httpConfig settings are: getConfirmationDeferDelay: 0ms, getConfirmationMaxRetries: 0, getConfirmationRetryDelay: 1000ms, getConfirmationExponentialBackoffDelay: 0ms, getConfirmationMaxExponentialBackoffDelay: 30000ms`, - ); - resolve(); - }); - }); + await Promise.race([ + new Promise((resolve) => { + returnPersistTransaction.on('error', (e: any) => { + expect(e.message).toBe( + 'The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object. The current httpConfig settings are: getConfirmationDeferDelay: 0ms, getConfirmationMaxRetries: 0, getConfirmationRetryDelay: 1000ms, getConfirmationExponentialBackoffDelay: 0ms, getConfirmationMaxExponentialBackoffDelay: 30000ms', + ); + resolve(); + }); + }), + new Promise((_, reject) => setTimeout(() => reject(new Error('Test timed out')), 5000)), + ]); }); }); }); From 9f81a7f42812d401781ea365ee30eacd184d7807 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Wed, 2 Oct 2024 01:25:37 -0400 Subject: [PATCH 21/24] Simplify and clarify the error message for better readability --- packages/request-client.js/src/http-data-access.ts | 2 +- packages/request-client.js/test/http-data-access.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index 162811a03..dd6adc48b 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -115,7 +115,7 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { let error: Error = e; if (e.status === 404) { error = new Error( - `The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object. The current httpConfig settings are: getConfirmationDeferDelay: ${this.httpConfig.getConfirmationDeferDelay}ms, getConfirmationMaxRetry: ${this.httpConfig.getConfirmationMaxRetry}, getConfirmationRetryDelay: ${this.httpConfig.getConfirmationRetryDelay}ms, getConfirmationExponentialBackoffDelay: ${this.httpConfig.getConfirmationExponentialBackoffDelay}ms, getConfirmationMaxExponentialBackoffDelay: ${this.httpConfig.getConfirmationMaxExponentialBackoffDelay}ms`, + `Timeout while confirming the Request was persisted. It is likely that the Request will be confirmed eventually. Recommend catching this error and using getConfirmedTransaction() to continue polling for confirmation. Consider adjusting the httpConfig settings on the RequestNetwork object to avoid future timeouts. Discourage calling persistTransaction() again as this would create a duplicate Request.`, ); } result.emit('error', error); diff --git a/packages/request-client.js/test/http-data-access.test.ts b/packages/request-client.js/test/http-data-access.test.ts index 7ed6b846a..e1ead9156 100644 --- a/packages/request-client.js/test/http-data-access.test.ts +++ b/packages/request-client.js/test/http-data-access.test.ts @@ -34,7 +34,7 @@ describe('HttpDataAccess', () => { new Promise((resolve) => { returnPersistTransaction.on('error', (e: any) => { expect(e.message).toBe( - 'The Request Network SDK timed-out while polling the Request Node to confirm that the Request was persisted successfully. It is likely that the persisted Request will be confirmed eventually. App Builders are discouraged from calling persistTransaction() a second time, which would create a duplicate Request. Instead, App Builders are recommended to catch this error and continue polling the Request Node using getConfirmedTransaction() or by calling the /getConfirmedTransaction endpoint. To avoid timeouts in the future, try adjusting the httpConfig values when instantiating the RequestNetwork object. The current httpConfig settings are: getConfirmationDeferDelay: 0ms, getConfirmationMaxRetries: 0, getConfirmationRetryDelay: 1000ms, getConfirmationExponentialBackoffDelay: 0ms, getConfirmationMaxExponentialBackoffDelay: 30000ms', + 'Timeout while confirming the Request was persisted. It is likely that the Request will be confirmed eventually. Recommend catching this error and using getConfirmedTransaction() to continue polling for confirmation. Consider adjusting the httpConfig settings on the RequestNetwork object to avoid future timeouts. Discourage calling persistTransaction() again as this would create a duplicate Request.', ); resolve(); }); From 4aca8712af8b824ab942661635ea5a87aa550731 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Wed, 2 Oct 2024 01:39:42 -0400 Subject: [PATCH 22/24] Assert that it actually throws the error after it times out --- packages/utils/test/retry.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/utils/test/retry.test.ts b/packages/utils/test/retry.test.ts index 7a4bd94ff..a2600af41 100644 --- a/packages/utils/test/retry.test.ts +++ b/packages/utils/test/retry.test.ts @@ -131,15 +131,13 @@ describe('Retry', () => { throw new Error(`threw`); }); - retry(throwFn, { + const retryPromise = retry(throwFn, { retryDelay: 0, // Exponential backoff starting at 1s, doubling after each retry, up to a maximum of 64s and max 7 retries, yielding a total of 8 call snad total timeout of 127s maxRetries: 7, exponentialBackoffDelay: 1000, // 1s maxExponentialBackoffDelay: 64000, // 64s - })().catch(() => { - // Do nothing - }); + })(); // Should call immediately (1 total calls, 0ms total elapsed) expect(throwFn).toHaveBeenCalledTimes(1); @@ -212,6 +210,8 @@ describe('Retry', () => { // No further retries jest.advanceTimersByTime(1000000000); await Promise.resolve(); + await expect(retryPromise).rejects.toThrow('threw'); + expect(throwFn).toHaveBeenCalledTimes(8); expect(Date.now()).toBe(1000127000); From 70e4a90a6c9a32b5dc7f70043aad7f84f73fc9f7 Mon Sep 17 00:00:00 2001 From: MantisClone Date: Wed, 2 Oct 2024 01:46:04 -0400 Subject: [PATCH 23/24] Improve error message per CodeRabbit comment --- packages/request-client.js/src/http-data-access.ts | 2 +- packages/request-client.js/test/http-data-access.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index dd6adc48b..030e1be7c 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -115,7 +115,7 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { let error: Error = e; if (e.status === 404) { error = new Error( - `Timeout while confirming the Request was persisted. It is likely that the Request will be confirmed eventually. Recommend catching this error and using getConfirmedTransaction() to continue polling for confirmation. Consider adjusting the httpConfig settings on the RequestNetwork object to avoid future timeouts. Discourage calling persistTransaction() again as this would create a duplicate Request.`, + `Timeout while confirming the Request was persisted. It is likely that the Request will be confirmed eventually. Catch this error and use getConfirmedTransaction() to continue polling for confirmation. Adjusting the httpConfig settings on the RequestNetwork object to avoid future timeouts. Avoid calling persistTransaction() again to prevent creating a duplicate Request.`, ); } result.emit('error', error); diff --git a/packages/request-client.js/test/http-data-access.test.ts b/packages/request-client.js/test/http-data-access.test.ts index e1ead9156..1afb56b96 100644 --- a/packages/request-client.js/test/http-data-access.test.ts +++ b/packages/request-client.js/test/http-data-access.test.ts @@ -34,7 +34,7 @@ describe('HttpDataAccess', () => { new Promise((resolve) => { returnPersistTransaction.on('error', (e: any) => { expect(e.message).toBe( - 'Timeout while confirming the Request was persisted. It is likely that the Request will be confirmed eventually. Recommend catching this error and using getConfirmedTransaction() to continue polling for confirmation. Consider adjusting the httpConfig settings on the RequestNetwork object to avoid future timeouts. Discourage calling persistTransaction() again as this would create a duplicate Request.', + 'Timeout while confirming the Request was persisted. It is likely that the Request will be confirmed eventually. Catch this error and use getConfirmedTransaction() to continue polling for confirmation. Adjusting the httpConfig settings on the RequestNetwork object to avoid future timeouts. Avoid calling persistTransaction() again to prevent creating a duplicate Request.', ); resolve(); }); From 9a8197c93ca604606355190622d4eb5cf3d4060f Mon Sep 17 00:00:00 2001 From: MantisClone Date: Wed, 2 Oct 2024 01:46:27 -0400 Subject: [PATCH 24/24] Ensure status exists before checking for equality --- packages/request-client.js/src/http-data-access.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/request-client.js/src/http-data-access.ts b/packages/request-client.js/src/http-data-access.ts index 030e1be7c..cf9cadd50 100644 --- a/packages/request-client.js/src/http-data-access.ts +++ b/packages/request-client.js/src/http-data-access.ts @@ -113,7 +113,7 @@ export default class HttpDataAccess implements DataAccessTypes.IDataAccess { }) .catch((e) => { let error: Error = e; - if (e.status === 404) { + if (e && 'status' in e && e.status === 404) { error = new Error( `Timeout while confirming the Request was persisted. It is likely that the Request will be confirmed eventually. Catch this error and use getConfirmedTransaction() to continue polling for confirmation. Adjusting the httpConfig settings on the RequestNetwork object to avoid future timeouts. Avoid calling persistTransaction() again to prevent creating a duplicate Request.`, );