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

Fix undefined property Error in executeAutoPipeline #1425

Merged
merged 1 commit into from
Aug 30, 2021

Conversation

TysonAndre
Copy link
Collaborator

@TysonAndre TysonAndre commented Aug 27, 2021

Seen in an uncaughtException handler with ioredis 4.27.7

The application having this issue has a constant&high volume of redis calls, and is using both Redis and Redis.Cluster clients to connect to different redis servers. So it might be related to redis cluster. Or newrelic, but I doubt it, processImmediate is something I'd guess would be from the setImmediate call?

This is happening throughout the day

err_type=uncaughtException, exiting cause=TypeError: Cannot read property 'Symbol(callbacks)' of undefined
    at Immediate.executeAutoPipeline (.../node_modules/ioredis/built/autoPipelining.js:34:31)
    at Shim.applySegment (.../node_modules/newrelic/lib/shim/shim.js:1430:20)
    at Immediate.wrapper (.../node_modules/newrelic/lib/shim/shim.js:2097:17)
    at processImmediate (internal/timers.js:463:21)

A similar error was among the errors reported in
#1248

I suspect the process.exec might call the callback synchronously in the
same tick in some cases, immediately deleting the autopipeline from
running pipelines?

i.e. my guess is that

  1. Something calls setImmediate for executeAutoPipeline
  2. Something else calls setImmediate
  3. The next tick starts
  4. immediateHandler calls executeAutoPipeline the first time, and the .exec somehow synchronously happens(same tick) and deletes the key from the Map
  5. immediateHandler calls executeAutoPipeline the second time for the same shard key, but because the shardKey was deleted, the fetched value was undefined

EDIT: This seems to have worked - the service remains running and stable with no issues after upgrading to the ioredis release including this fix (4.27.9) (and Cannot read property 'Symbol(callbacks)' of undefined stopped)

Seen in an uncaughtException handler.

```
err_type=uncaughtException, exiting cause=TypeError: Cannot read property 'Symbol(callbacks)' of undefined
    at Immediate.executeAutoPipeline (.../node_modules/ioredis/built/autoPipelining.js:34:31)
    at Shim.applySegment (.../node_modules/newrelic/lib/shim/shim.js:1430:20)
    at Immediate.wrapper (.../node_modules/newrelic/lib/shim/shim.js:2097:17)
    at processImmediate (internal/timers.js:463:21)
```

A similar error was among the errors reported in
redis#1248

I suspect the process.exec might call the callback synchronously in the
same tick in some cases, immediately deleting the autopipeline from
running pipelines?
@TysonAndre TysonAndre changed the title Fix undefined property warning in executeAutoPipeline Fix undefined property Error in executeAutoPipeline Aug 27, 2021
@TysonAndre
Copy link
Collaborator Author

This is ready for review. Any comments?

@luin luin merged commit f898672 into redis:master Aug 30, 2021
ioredis-robot pushed a commit that referenced this pull request Aug 30, 2021
## [4.27.9](v4.27.8...v4.27.9) (2021-08-30)

### Bug Fixes

* Fix undefined property warning in executeAutoPipeline ([#1425](#1425)) ([f898672](f898672))
* improve proto checking for hgetall [skip ci] ([#1418](#1418)) ([cba83cb](cba83cb))
@ioredis-robot
Copy link
Collaborator

🎉 This PR is included in version 4.27.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

@TysonAndre TysonAndre deleted the fix-_autoPipelines-undefined branch August 30, 2021 21:59
janus-dev87 added a commit to janus-dev87/ioredis-work that referenced this pull request Mar 1, 2024
## [4.27.9](redis/ioredis@v4.27.8...v4.27.9) (2021-08-30)

### Bug Fixes

* Fix undefined property warning in executeAutoPipeline ([#1425](redis/ioredis#1425)) ([f898672](redis/ioredis@f898672))
* improve proto checking for hgetall [skip ci] ([#1418](redis/ioredis#1418)) ([cba83cb](redis/ioredis@cba83cb))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants