From 2b4059ef4f519f88ea0bdbd278b39ef9ba301e07 Mon Sep 17 00:00:00 2001 From: Josh Mock Date: Fri, 21 Jun 2024 12:30:44 -0500 Subject: [PATCH 1/8] Do not retry on timeout by default --- lib/Transport.js | 9 +++++++++ test/unit/transport.test.js | 2 ++ 2 files changed, 11 insertions(+) diff --git a/lib/Transport.js b/lib/Transport.js index d30f6347a..5337256f4 100644 --- a/lib/Transport.js +++ b/lib/Transport.js @@ -57,6 +57,7 @@ class Transport { this.serializer = opts.serializer this.maxRetries = opts.maxRetries this.requestTimeout = toMs(opts.requestTimeout) + this.retryOnTimeout = opts.retryOnTimeout != null ? opts.retryOnTimeout : false this.suggestCompression = opts.suggestCompression === true this.compression = opts.compression || false this.context = opts.context || null @@ -220,6 +221,13 @@ class Transport { }) } + // do not retry timeout errors by default + if (err.name === 'TimeoutError' && this.retryOnTimeout === false) { + err.meta = result + this.emit('response', err, result) + return callback(err, result) + } + // retry logic if (meta.attempts < maxRetries) { meta.attempts++ @@ -229,6 +237,7 @@ class Transport { } } + err.meta = result this.emit('response', err, result) return callback(err, result) diff --git a/test/unit/transport.test.js b/test/unit/transport.test.js index e1a6aaa9f..5f183a2e9 100644 --- a/test/unit/transport.test.js +++ b/test/unit/transport.test.js @@ -1025,6 +1025,7 @@ test('Retry mechanism and abort', t => { serializer: new Serializer(), maxRetries: 2, requestTimeout: 100, + retryOnTimeout: true, sniffInterval: false, sniffOnStart: false }) @@ -2203,6 +2204,7 @@ test('Compress request', t => { serializer: new Serializer(), maxRetries: 3, requestTimeout: 250, + retryOnTimeout: true, sniffInterval: false, sniffOnStart: false }) From 7702e80087c88087716ac1f764830f70810cf0ad Mon Sep 17 00:00:00 2001 From: Josh Mock Date: Fri, 21 Jun 2024 12:46:43 -0500 Subject: [PATCH 2/8] Stop testing on Node.js 12 No longer available on actions/setup-node --- .github/workflows/nodejs.yml | 244 +++++++++++++++++------------------ 1 file changed, 122 insertions(+), 122 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 43181ce94..1e4bb2057 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -9,36 +9,36 @@ jobs: strategy: matrix: - node-version: [12.x, 14.x, 16.x] + node-version: [14.x, 16.x] os: [ubuntu-latest, windows-latest, macOS-latest] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v2 - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 - with: - node-version: ${{ matrix.node-version }} + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node-version }} - - name: Install - run: | - npm install + - name: Install + run: | + npm install - - name: Lint - run: | - npm run lint + - name: Lint + run: | + npm run lint - - name: Unit test - run: | - npm run test:unit + - name: Unit test + run: | + npm run test:unit - - name: Acceptance test - run: | - npm run test:acceptance + - name: Acceptance test + run: | + npm run test:acceptance - - name: Type Definitions - run: | - npm run test:types + - name: Type Definitions + run: | + npm run test:types helpers-integration-test: name: Helpers integration test @@ -46,99 +46,99 @@ jobs: strategy: matrix: - node-version: [12.x, 14.x, 16.x] + node-version: [14.x, 16.x] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v2 - - name: Configure sysctl limits - run: | - sudo swapoff -a - sudo sysctl -w vm.swappiness=1 - sudo sysctl -w fs.file-max=262144 - sudo sysctl -w vm.max_map_count=262144 + - name: Configure sysctl limits + run: | + sudo swapoff -a + sudo sysctl -w vm.swappiness=1 + sudo sysctl -w fs.file-max=262144 + sudo sysctl -w vm.max_map_count=262144 - - name: Runs Elasticsearch - uses: elastic/elastic-github-actions/elasticsearch@master - with: - stack-version: 7.16-SNAPSHOT + - name: Runs Elasticsearch + uses: elastic/elastic-github-actions/elasticsearch@master + with: + stack-version: 7.16-SNAPSHOT - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 - with: - node-version: ${{ matrix.node-version }} + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node-version }} - - name: Install - run: | - npm install + - name: Install + run: | + npm install - - name: Integration test - run: | - npm run test:integration:helpers + - name: Integration test + run: | + npm run test:integration:helpers bundler-support: name: Bundler support runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - - name: Configure sysctl limits - run: | - sudo swapoff -a - sudo sysctl -w vm.swappiness=1 - sudo sysctl -w fs.file-max=262144 - sudo sysctl -w vm.max_map_count=262144 - - - name: Runs Elasticsearch - uses: elastic/elastic-github-actions/elasticsearch@master - with: - stack-version: 7.16-SNAPSHOT - - - name: Use Node.js 14.x - uses: actions/setup-node@v1 - with: - node-version: 14.x - - - name: Install - run: | - npm install - npm install --prefix test/bundlers/parcel-test - npm install --prefix test/bundlers/rollup-test - npm install --prefix test/bundlers/webpack-test - - - name: Build - run: | - npm run build --prefix test/bundlers/parcel-test - npm run build --prefix test/bundlers/rollup-test - npm run build --prefix test/bundlers/webpack-test - - - name: Run bundle - run: | - npm start --prefix test/bundlers/parcel-test - npm start --prefix test/bundlers/rollup-test - npm start --prefix test/bundlers/webpack-test + - uses: actions/checkout@v2 + + - name: Configure sysctl limits + run: | + sudo swapoff -a + sudo sysctl -w vm.swappiness=1 + sudo sysctl -w fs.file-max=262144 + sudo sysctl -w vm.max_map_count=262144 + + - name: Runs Elasticsearch + uses: elastic/elastic-github-actions/elasticsearch@master + with: + stack-version: 7.16-SNAPSHOT + + - name: Use Node.js 14.x + uses: actions/setup-node@v1 + with: + node-version: 14.x + + - name: Install + run: | + npm install + npm install --prefix test/bundlers/parcel-test + npm install --prefix test/bundlers/rollup-test + npm install --prefix test/bundlers/webpack-test + + - name: Build + run: | + npm run build --prefix test/bundlers/parcel-test + npm run build --prefix test/bundlers/rollup-test + npm run build --prefix test/bundlers/webpack-test + + - name: Run bundle + run: | + npm start --prefix test/bundlers/parcel-test + npm start --prefix test/bundlers/rollup-test + npm start --prefix test/bundlers/webpack-test mock-support: name: Mock support runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v2 - - name: Use Node.js 14.x - uses: actions/setup-node@v1 - with: - node-version: 14.x + - name: Use Node.js 14.x + uses: actions/setup-node@v1 + with: + node-version: 14.x - - name: Install - run: | - npm install - npm install --prefix test/mock + - name: Install + run: | + npm install + npm install --prefix test/mock - - name: Run test - run: | - npm test --prefix test/mock + - name: Run test + run: | + npm test --prefix test/mock code-coverage: name: Code coverage @@ -149,30 +149,30 @@ jobs: node-version: [14.x] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v2 - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 - with: - node-version: ${{ matrix.node-version }} + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node-version }} - - name: Install - run: | - npm install + - name: Install + run: | + npm install - - name: Code coverage report - run: | - npm run test:coverage-report + - name: Code coverage report + run: | + npm run test:coverage-report - - name: Upload coverage to Codecov - uses: codecov/codecov-action@v1 - with: - file: ./coverage.lcov - fail_ci_if_error: true + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v1 + with: + file: ./coverage.lcov + fail_ci_if_error: true - - name: Code coverage 100% - run: | - npm run test:coverage-100 + - name: Code coverage 100% + run: | + npm run test:coverage-100 license: name: License check @@ -183,17 +183,17 @@ jobs: node-version: [14.x] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v2 - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 - with: - node-version: ${{ matrix.node-version }} + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node-version }} - - name: Install - run: | - npm install + - name: Install + run: | + npm install - - name: License checker - run: | - npm run license-checker + - name: License checker + run: | + npm run license-checker From e808f3d68d3b5c6872c0dc35f222768ca94ee7ad Mon Sep 17 00:00:00 2001 From: Josh Mock Date: Fri, 21 Jun 2024 12:49:13 -0500 Subject: [PATCH 3/8] Node.js 14.x is no longer available for testing either --- .github/workflows/nodejs.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 1e4bb2057..b39dc316c 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -9,7 +9,7 @@ jobs: strategy: matrix: - node-version: [14.x, 16.x] + node-version: [16.x, 18.x, 20.x] os: [ubuntu-latest, windows-latest, macOS-latest] steps: @@ -46,7 +46,7 @@ jobs: strategy: matrix: - node-version: [14.x, 16.x] + node-version: [16.x, 18.x, 20.x] steps: - uses: actions/checkout@v2 From d4e1f78d1bf111735026827f218844dc284d325c Mon Sep 17 00:00:00 2001 From: Josh Mock Date: Fri, 21 Jun 2024 13:07:24 -0500 Subject: [PATCH 4/8] Fix linter issue --- lib/Transport.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Transport.js b/lib/Transport.js index 5337256f4..ece84b541 100644 --- a/lib/Transport.js +++ b/lib/Transport.js @@ -237,7 +237,6 @@ class Transport { } } - err.meta = result this.emit('response', err, result) return callback(err, result) From 4d4196dee65bdb3f93e4a89361bd2681cc41d982 Mon Sep 17 00:00:00 2001 From: Josh Mock Date: Fri, 21 Jun 2024 13:54:42 -0500 Subject: [PATCH 5/8] Update acceptance tests --- lib/Transport.js | 2 +- test/acceptance/events-order.test.js | 26 +++++++++++++++++--------- test/acceptance/product-check.test.js | 3 ++- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/lib/Transport.js b/lib/Transport.js index ece84b541..f499a8783 100644 --- a/lib/Transport.js +++ b/lib/Transport.js @@ -222,7 +222,7 @@ class Transport { } // do not retry timeout errors by default - if (err.name === 'TimeoutError' && this.retryOnTimeout === false) { + if (err.name === 'TimeoutError' && this.retryOnTimeout !== true) { err.meta = result this.emit('response', err, result) return callback(err, result) diff --git a/test/acceptance/events-order.test.js b/test/acceptance/events-order.test.js index 335fd4ba8..b51e8b835 100644 --- a/test/acceptance/events-order.test.js +++ b/test/acceptance/events-order.test.js @@ -88,7 +88,8 @@ test('Connection error', t => { const client = new Client({ node: 'http://localhost:9200', Connection: MockConnectionError, - maxRetries: 1 + maxRetries: 1, + retryOnTimeout: true }) const order = [ @@ -124,18 +125,18 @@ test('Connection error', t => { }) test('TimeoutError error', t => { - t.plan(10) + t.plan(8) const client = new Client({ node: 'http://localhost:9200', Connection: MockConnectionTimeout, - maxRetries: 1 + maxRetries: 1, + retryOnTimeout: true }) const order = [ events.SERIALIZATION, events.REQUEST, - events.REQUEST, events.RESPONSE ] @@ -170,7 +171,8 @@ test('RequestAbortedError error', t => { const client = new Client({ node: 'http://localhost:9200', Connection: MockConnectionTimeout, - maxRetries: 1 + maxRetries: 1, + retryOnTimeout: true }) const order = [ @@ -221,7 +223,8 @@ test('ResponseError error (no retry)', t => { const client = new Client({ node: 'http://localhost:9200', Connection: MockConnection, - maxRetries: 1 + maxRetries: 1, + retryOnTimeout: true }) const order = [ @@ -316,7 +319,7 @@ test('Serialization Error', t => { const client = new Client({ node: 'http://localhost:9200', Connection: MockConnection, - maxRetries: 1 + maxRetries: 1, }) const order = [ @@ -373,7 +376,8 @@ test('Deserialization Error', t => { const client = new Client({ node: 'http://localhost:9200', Connection: MockConnection, - maxRetries: 1 + maxRetries: 1, + retryOnTimeout: true }) const order = [ @@ -423,7 +427,11 @@ test('Socket destroyed while reading the body', t => { } buildServer(handler, ({ port }, server) => { - const client = new Client({ node: `http://localhost:${port}`, maxRetries: 1 }) + const client = new Client({ + node: `http://localhost:${port}`, + maxRetries: 1, + retryOnTimeout: true + }) const order = [ events.SERIALIZATION, diff --git a/test/acceptance/product-check.test.js b/test/acceptance/product-check.test.js index 2ed7cec88..3f788d6ea 100644 --- a/test/acceptance/product-check.test.js +++ b/test/acceptance/product-check.test.js @@ -680,7 +680,8 @@ test('TimeoutError', t => { const client = new Client({ node: 'http://localhost:9200', Connection: MockConnectionTimeout, - maxRetries: 0 + maxRetries: 0, + retryOnTimeout: true }) client.on('request', (err, event) => { From 8986d72ab544782d14a66b088153971bef5d0d32 Mon Sep 17 00:00:00 2001 From: Josh Mock Date: Fri, 21 Jun 2024 13:56:08 -0500 Subject: [PATCH 6/8] Add retryOnTimeout to type defs --- lib/Transport.d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Transport.d.ts b/lib/Transport.d.ts index 25b770fdb..6d3c06b24 100644 --- a/lib/Transport.d.ts +++ b/lib/Transport.d.ts @@ -49,6 +49,7 @@ interface TransportOptions { serializer: Serializer; maxRetries: number; requestTimeout: number | string; + retryOnTimeout?: boolean; suggestCompression?: boolean; compression?: 'gzip'; sniffInterval?: number; From 4d6cd7a350b9cad1cfbf6276a870f69be743a33b Mon Sep 17 00:00:00 2001 From: Josh Mock Date: Fri, 21 Jun 2024 13:57:27 -0500 Subject: [PATCH 7/8] Linter cleanup --- test/acceptance/events-order.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/events-order.test.js b/test/acceptance/events-order.test.js index b51e8b835..c8fa20d38 100644 --- a/test/acceptance/events-order.test.js +++ b/test/acceptance/events-order.test.js @@ -319,7 +319,7 @@ test('Serialization Error', t => { const client = new Client({ node: 'http://localhost:9200', Connection: MockConnection, - maxRetries: 1, + maxRetries: 1 }) const order = [ From 9ddf89ba55c32038ed34539ba1fc6bafa5c860a6 Mon Sep 17 00:00:00 2001 From: Josh Mock Date: Fri, 28 Jun 2024 12:12:47 -0500 Subject: [PATCH 8/8] Drop code coverage step from Github action --- .github/workflows/nodejs.yml | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index b39dc316c..45897aec0 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -140,40 +140,6 @@ jobs: run: | npm test --prefix test/mock - code-coverage: - name: Code coverage - runs-on: ubuntu-latest - - strategy: - matrix: - node-version: [14.x] - - steps: - - uses: actions/checkout@v2 - - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 - with: - node-version: ${{ matrix.node-version }} - - - name: Install - run: | - npm install - - - name: Code coverage report - run: | - npm run test:coverage-report - - - name: Upload coverage to Codecov - uses: codecov/codecov-action@v1 - with: - file: ./coverage.lcov - fail_ci_if_error: true - - - name: Code coverage 100% - run: | - npm run test:coverage-100 - license: name: License check runs-on: ubuntu-latest