Upgrade from 3.1.2 to 4 has broken message delivery
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 17.6k
- Forks
- 2k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 40
Description
Environment:
- Node.js Version: 17
- Redis Server Version: v=6.2.3
- Node Redis Version: 4.1.0
- Platform: Dev platform: Release Linux Mint 20.3 Una 64-bit Prod platform: Any Linux
This is a continuation from https://github.com/redis/node-redis/issues/1870 (@leibale you were working on that). We had to release with version 3.1.2 (our existing version) as we couldn't keep consumers waiting any longer. Now circling back to work out what the actual issue is in hope that we can upgrade node redis.
At this stage it's hard to tell. We're not getting any errors. We've upgraded node redis to the latest now (4.1.0), read the Changelog again, been over the v3 to v4 MIgration Guide many times and made the required changes, also in line with createClient configuration.
This is our consolidated diff between the code that works with 3.1.2 (main branch) and the upgraded code to satisfy 4.1.0 requirements (binarymist/upgrade-incl-redis branch).
The most obvious place to be focussing on is in the testerWatcher.
We have two different communication techniques from our CLI to the orchestrator (Server Sent Events (sse), and long polling (lp)). The sse appears to be working fine with the upgrade of node redis, but the lp is not.
The change you see in the get.js is only applicable to sse so I don't think there's any issue in get.js.
We have multiple Test Sessions (each relly on their specific redis channel. The first one seems to work). The changes to the orchestrator.js are unlikely to be an issue if one of the redis channels is working, which leaves our foucs to the testerWatcher... and specifically pollTesterMessages.
There is either something we've somehow missed changing with the upgrade, although I can't see this being the case as we've gone over your upgrade path a good number of times over the last few months. Or there is a bug in node redis. Either way it needs to be fixed in order for us to upgrade node redis.
A high level overview of our architecture can be seen here.
Thanks.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the testerWatcher, especially pollTesterMessages, and compare the working 3.1.2 code with the 4.1.0 upgrade diff; get.js is reported as relevant only to SSE, while orchestrator.js appears less likely. Trace Redis delivery across the separate test-session channels and long-polling path; done means all sessions receive messages under node-redis 4.1.0 without silent failures.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, redis
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100