Skip to content

Conversation

@trentm
Copy link
Member

@trentm trentm commented Oct 25, 2022

client.set('foo') fails on 4.1, but not on 4.0.


The redis TAV tests failed today: https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-nodejs%2Fapm-agent-nodejs-mbp/detail/main/464/pipeline

[2022-10-25T07:01:48.084Z] node_tests_1  | -- installing ["[email protected]"]
[2022-10-25T07:01:57.820Z] node_tests_1  | -- running test "node test/instrumentation/modules/redis.test.js" with redis
...
# redis client error
not ok 61 no response expected
  ---
    operator: fail
    at: <anonymous> (/Users/trentm/el/apm-agent-nodejs4/test/instrumentation/modules/redis.test.js:167:7)
    stack: |-
      Error: no response expected
          at Test.assert [as _assert] (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:312:48)
          at Test.bound [as _assert] (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:95:17)
          at Test.fail (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:406:7)
          at Test.bound [as fail] (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:95:17)
          at /Users/trentm/el/apm-agent-nodejs4/test/instrumentation/modules/redis.test.js:167:7
          at processTicksAndRejections (node:internal/process/task_queues:96:5)
  ...
ok 62 got 1 transaction
ok 63 got 1 span
not ok 64 got 1 error
  ---
    operator: equal
    expected: 1
    actual:   0
    at: <anonymous> (/Users/trentm/el/apm-agent-nodejs4/test/instrumentation/modules/redis.test.js:141:7)
    stack: |-
      Error: got 1 error
          at Test.assert [as _assert] (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:312:48)
          at Test.bound [as _assert] (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:95:17)
          at Test.strictEqual (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:476:7)
          at Test.bound [as equal] (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:95:17)
          at /Users/trentm/el/apm-agent-nodejs4/test/instrumentation/modules/redis.test.js:141:7
          at Timeout._onTimeout (/Users/trentm/el/apm-agent-nodejs4/test/_mock_http_client.js:80:7)
          at listOnTimeout (node:internal/timers:559:17)
          at processTimers (node:internal/timers:502:7)
  ...
ok 65 span.name
ok 66 span.parent_id
not ok 67 span.outcome
  ---
    operator: equal
    expected: 'failure'
    actual:   'success'
    at: <anonymous> (/Users/trentm/el/apm-agent-nodejs4/test/instrumentation/modules/redis.test.js:144:7)
    stack: |-
      Error: span.outcome
          at Test.assert [as _assert] (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:312:48)
          at Test.bound [as _assert] (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:95:17)
          at Test.strictEqual (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:476:7)
          at Test.bound [as equal] (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:95:17)
          at /Users/trentm/el/apm-agent-nodejs4/test/instrumentation/modules/redis.test.js:144:7
          at Timeout._onTimeout (/Users/trentm/el/apm-agent-nodejs4/test/_mock_http_client.js:80:7)
          at listOnTimeout (node:internal/timers:559:17)
          at processTimers (node:internal/timers:502:7)
  ...
/Users/trentm/el/apm-agent-nodejs4/test/instrumentation/modules/redis.test.js:145
    t.equal(data.errors[0].transaction_id, data.transactions[0].id, 'error.transaction_id')
                           ^

TypeError: Cannot read properties of undefined (reading 'transaction_id')
    at /Users/trentm/el/apm-agent-nodejs4/test/instrumentation/modules/redis.test.js:145:28
    at Timeout._onTimeout (/Users/trentm/el/apm-agent-nodejs4/test/_mock_http_client.js:80:7)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

The issue is that our chosen way to get a client error (to test error capture) doesn't result in an error on redis 4.0.x:

> require('redis/package.json').version
'4.0.6'
> var redis = require('redis')
undefined
> var client = redis.createClient()
undefined
> client.connect()
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 55,
  [Symbol(trigger_async_id_symbol)]: 5,
  [Symbol(destroyed)]: { destroyed: false }
}
> client.set('foo').then(r => { console.log('good', r) }).catch(e => {console.log('bad', e)})
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 85,
  [Symbol(trigger_async_id_symbol)]: 84,
  [Symbol(destroyed)]: { destroyed: false }
}
> good OK

It does on redis.4.1.x:

> require('redis/package.json').version
'4.1.0'
> var redis = require('redis')
undefined
> var client = redis.createClient()
undefined
> client.connect()
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 32,
  [Symbol(trigger_async_id_symbol)]: 5,
  [Symbol(destroyed)]: { destroyed: false }
}
> client.set('foo').then(r => { console.log('good', r) }).catch(e => {console.log('bad', e)})
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 58,
  [Symbol(trigger_async_id_symbol)]: 57,
  [Symbol(destroyed)]: { destroyed: false }
}
> bad TypeError: Invalid argument type
    at encodeCommand (/Users/trentm/el/apm-agent-nodejs4/node_modules/@redis/client/dist/lib/client/RESP2/encoder.js:20:19)
    at RedisCommandsQueue.getCommandToSend (/Users/trentm/el/apm-agent-nodejs4/node_modules/@redis/client/dist/lib/client/commands-queue.js:187:45)
    at Commander._RedisClient_tick (/Users/trentm/el/apm-agent-nodejs4/node_modules/@redis/client/dist/lib/client/index.js:429:76)
    at Commander._RedisClient_sendCommand (/Users/trentm/el/apm-agent-nodejs4/node_modules/@redis/client/dist/lib/client/index.js:413:82)
    at Commander.commandsExecutor (/Users/trentm/el/apm-agent-nodejs4/node_modules/@redis/client/dist/lib/client/index.js:167:154)
    at Commander.BaseClass.<computed> [as set] (/Users/trentm/el/apm-agent-nodejs4/node_modules/@redis/client/dist/lib/commander.js:8:29)
    at REPL5:1:8
    at Script.runInThisContext (node:vm:129:12)
    at REPLServer.defaultEval (node:repl:566:29)
    at bound (node:domain:421:15)

This just skips that particular test on 4.0.x.

client.set('foo') fails on 4.1, but not on 4.0.
@trentm trentm self-assigned this Oct 25, 2022
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Oct 25, 2022
@trentm
Copy link
Member Author

trentm commented Oct 25, 2022

run module tests for redis

@ghost
Copy link

ghost commented Oct 25, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-10-25T23:21:03.011+0000

  • Duration: 34 min 49 sec

Test stats 🧪

Test Results
Failed 0
Passed 269774
Skipped 0
Total 269774

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run module tests for <modules> : Run TAV tests for one or more modules, where <modules> can be either a comma separated list of modules (e.g. memcached,redis) or the string literal ALL to test all modules

  • run benchmark tests : Run the benchmark test only.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@trentm trentm requested a review from astorm October 27, 2022 04:06
@trentm trentm merged commit b9b2d18 into main Oct 27, 2022
@trentm trentm deleted the trentm/fix-redis-tav-tests branch October 27, 2022 15:28
PeterEinberger pushed a commit to fpm-git/apm-agent-nodejs that referenced this pull request Aug 20, 2024
client.set('foo') fails on 4.1, but not on 4.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-nodejs Make available for APM Agents project planning.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants