fix: Redis 4 does not reconnect after unhandled error #8706

Vortec4800 · 2023-08-01T16:44:45Z

Pull Request

Report security issues confidentially.
Any contribution is under this license.
Link this pull request to an issue.

Issue

Closes: #8705

Approach

Adds mostly empty event handlers to the Redis client, which is now required for Redis 4.

Tasks

Add changes to documentation (guides, repository pages, code comments)
Add security check

parse-github-assistant · 2023-08-01T16:44:48Z

Thanks for opening this pull request!

mtrezza

Could you add a test that demonstrates the issue before adding the fix and the passes with the fix?

It seems strange that adding empty handlers fixed this. Could you add a Redis docs reference that describes this requirement?

This makes me think which version of Redis we are actually testing with. It seems we need a compatibility table to the README and test with multiple versions just like we do with MongoDB. In our CI we are currently not specifying a Redis version, so it's using the latest version from Docker, which I think is Redis 7.0.12. Without testing with explicit versions I don't think we should merge this PR because the change may break compatibility with another Redis version.

Vortec4800 · 2023-08-03T13:10:14Z

I agree it's strange. Here is the Redis issue that led me to the solution for reference.

redis/node-redis#2032

There's also some discussion on how to reliably recreate the issue, it seems that hosted Redis servers like Digital Ocean or Heroku do this pretty often. I don't think the server version matters for this, only the client version, and it seems these handlers didn't change between version 3.x of the client and 4.x - only their requirement to exist.

From that issue it links to the main readme that has a bit more information, and that also confirms the need for event handlers and the error thrown if they aren't handled: https://github.com/redis/node-redis#events . It seems we technically only need to add an error handler and not the other ones from the PR, so that could potentially be optimized.

I'm not sure offhand how to test this scenario, but let me investigate and see if there's something I can put together for that.

ChrisGrant · 2023-08-10T15:35:34Z

For what it is worth, I am seeing the same issue on Heroku with the latest version of parse-server (6.2.1).

Redis version is 6.2.13.

codecov · 2023-08-14T12:46:20Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (42929e0) 94.32% compared to head (7b13b56) 94.33%.

❗ Current head 7b13b56 differs from pull request most recent head 83aba07. Consider uploading reports for the commit 83aba07 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha    #8706      +/-   ##
==========================================
+ Coverage   94.32%   94.33%   +0.01%     
==========================================
  Files         185      185              
  Lines       14762    14766       +4     
==========================================
+ Hits        13924    13930       +6     
+ Misses        838      836       -2

Files Changed	Coverage Δ
src/Adapters/Cache/RedisCacheAdapter.js	`92.00% <100.00%> (+0.69%)`	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mtrezza · 2023-08-15T21:29:01Z

@Vortec4800 Did you find a way to add a test for this? If not, we may as well merge as is; if you could just take a look at the Lint error. We just have to be sure that this is not a breaking change in a certain scenario.

Vortec4800 · 2023-08-20T17:29:12Z

I'm sorry - work got busy and I haven't had a chance to look at this further. I did just add a quick change that should address the lint error, and log client errors via the internal logger. Take a look and let me know if that's appropriate. I can also add info logs to the other client events if you think that would be a good idea.

mtrezza · 2023-08-21T17:43:28Z

Thanks for the commits, let's see if the CI passes...

Signed-off-by: Cory Imdieke <[email protected]>

mtrezza · 2023-08-29T23:02:50Z

It's strange that this one Postgres job keeps failing in the CI, I've restarted it several times. Restarted again.

# [6.3.0-alpha.8](6.3.0-alpha.7...6.3.0-alpha.8) (2023-08-30) ### Bug Fixes * Redis 4 does not reconnect after unhandled error ([#8706](#8706)) ([2b3d4e5](2b3d4e5))

parseplatformorg · 2023-08-30T00:53:53Z

🎉 This change has been released in version 6.3.0-alpha.8

# [6.4.0-beta.1](6.3.0...6.4.0-beta.1) (2023-09-16) ### Bug Fixes * Parse Server option `fileUpload.fileExtensions` does not work with an array of extensions ([#8688](#8688)) ([6a4a00c](6a4a00c)) * Redis 4 does not reconnect after unhandled error ([#8706](#8706)) ([2b3d4e5](2b3d4e5)) * Remove config logging when launching Parse Server via CLI ([#8710](#8710)) ([ae68f0c](ae68f0c)) * Server does not start via CLI when `auth` option is set ([#8666](#8666)) ([4e2000b](4e2000b)) ### Features * Add conditional email verification via dynamic Parse Server options `verifyUserEmails`, `sendUserEmailVerification` that now accept functions ([#8425](#8425)) ([44acd6d](44acd6d)) * Add property `Parse.Server.version` to determine current version of Parse Server in Cloud Code ([#8670](#8670)) ([a9d376b](a9d376b)) * Add TOTP authentication adapter ([#8457](#8457)) ([cc079a4](cc079a4)) ### Performance Improvements * Improve performance of recursive pointer iterations ([#8741](#8741)) ([45a3ed0](45a3ed0))

parseplatformorg · 2023-09-16T02:54:00Z

🎉 This change has been released in version 6.4.0-beta.1

# [6.4.0-alpha.1](6.3.0...6.4.0-alpha.1) (2023-09-20) ### Bug Fixes * Parse Server option `fileUpload.fileExtensions` does not work with an array of extensions ([#8688](#8688)) ([6a4a00c](6a4a00c)) * Redis 4 does not reconnect after unhandled error ([#8706](#8706)) ([2b3d4e5](2b3d4e5)) * Remove config logging when launching Parse Server via CLI ([#8710](#8710)) ([ae68f0c](ae68f0c)) * Server does not start via CLI when `auth` option is set ([#8666](#8666)) ([4e2000b](4e2000b)) ### Features * Add conditional email verification via dynamic Parse Server options `verifyUserEmails`, `sendUserEmailVerification` that now accept functions ([#8425](#8425)) ([44acd6d](44acd6d)) * Add context to Cloud Code Triggers `beforeLogin` and `afterLogin` ([#8724](#8724)) ([a9c34ef](a9c34ef)) * Add property `Parse.Server.version` to determine current version of Parse Server in Cloud Code ([#8670](#8670)) ([a9d376b](a9d376b)) * Add TOTP authentication adapter ([#8457](#8457)) ([cc079a4](cc079a4)) ### Performance Improvements * Improve performance of recursive pointer iterations ([#8741](#8741)) ([45a3ed0](45a3ed0))

parseplatformorg · 2023-09-20T08:50:41Z

🎉 This change has been released in version 6.4.0-alpha.1

# [6.4.0](6.3.1...6.4.0) (2023-11-16) ### Bug Fixes * Parse Server option `fileUpload.fileExtensions` does not work with an array of extensions ([#8688](#8688)) ([6a4a00c](6a4a00c)) * Redis 4 does not reconnect after unhandled error ([#8706](#8706)) ([2b3d4e5](2b3d4e5)) * Remove config logging when launching Parse Server via CLI ([#8710](#8710)) ([ae68f0c](ae68f0c)) * Server does not start via CLI when `auth` option is set ([#8666](#8666)) ([4e2000b](4e2000b)) ### Features * Add conditional email verification via dynamic Parse Server options `verifyUserEmails`, `sendUserEmailVerification` that now accept functions ([#8425](#8425)) ([44acd6d](44acd6d)) * Add property `Parse.Server.version` to determine current version of Parse Server in Cloud Code ([#8670](#8670)) ([a9d376b](a9d376b)) * Add TOTP authentication adapter ([#8457](#8457)) ([cc079a4](cc079a4)) ### Performance Improvements * Improve performance of recursive pointer iterations ([#8741](#8741)) ([45a3ed0](45a3ed0))

parseplatformorg · 2023-11-16T15:28:00Z

🎉 This change has been released in version 6.4.0

mtrezza changed the title ~~Fixed Redis 4 Reconnect Issue~~ fix: Redis 4 does not reconnect after unhandled error Aug 3, 2023

mtrezza requested changes Aug 3, 2023

View reviewed changes

Vortec4800 force-pushed the patch-1 branch from 3121fb2 to 13aec7e Compare August 20, 2023 17:27

Vortec4800 added 4 commits August 21, 2023 13:37

Fixed Redis 4 Reconnect Issue

45a990b

Signed-off-by: Cory Imdieke <[email protected]>

Cleaned up spacing

bf3fd63

Signed-off-by: Cory Imdieke <[email protected]>

Removed logging in favor of auto reconnect to match old behavior

877e77e

Signed-off-by: Cory Imdieke <[email protected]>

Update to fix lint error

83aba07

Signed-off-by: Cory Imdieke <[email protected]>

Vortec4800 force-pushed the patch-1 branch from 13aec7e to 83aba07 Compare August 21, 2023 18:37

mtrezza merged commit 2b3d4e5 into parse-community:alpha Aug 30, 2023
24 checks passed

parseplatformorg added the state:released-alpha Released as alpha version label Aug 30, 2023

Vortec4800 deleted the patch-1 branch August 30, 2023 13:19

parseplatformorg added the state:released-beta Released as beta version label Sep 16, 2023

parseplatformorg added the state:released Released as stable version label Nov 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Redis 4 does not reconnect after unhandled error #8706

fix: Redis 4 does not reconnect after unhandled error #8706

Vortec4800 commented Aug 1, 2023

parse-github-assistant bot commented Aug 1, 2023

mtrezza left a comment •

edited

Loading

Vortec4800 commented Aug 3, 2023 •

edited

Loading

ChrisGrant commented Aug 10, 2023

codecov bot commented Aug 14, 2023 •

edited

Loading

mtrezza commented Aug 15, 2023 •

edited

Loading

Vortec4800 commented Aug 20, 2023

mtrezza commented Aug 21, 2023

mtrezza commented Aug 29, 2023

parseplatformorg commented Aug 30, 2023

parseplatformorg commented Sep 16, 2023

parseplatformorg commented Sep 20, 2023

parseplatformorg commented Nov 16, 2023

fix: Redis 4 does not reconnect after unhandled error #8706

fix: Redis 4 does not reconnect after unhandled error #8706

Conversation

Vortec4800 commented Aug 1, 2023

Pull Request

Issue

Approach

Tasks

parse-github-assistant bot commented Aug 1, 2023

Thanks for opening this pull request!

mtrezza left a comment • edited Loading

Choose a reason for hiding this comment

Vortec4800 commented Aug 3, 2023 • edited Loading

ChrisGrant commented Aug 10, 2023

codecov bot commented Aug 14, 2023 • edited Loading

Codecov Report

mtrezza commented Aug 15, 2023 • edited Loading

Vortec4800 commented Aug 20, 2023

mtrezza commented Aug 21, 2023

mtrezza commented Aug 29, 2023

parseplatformorg commented Aug 30, 2023

parseplatformorg commented Sep 16, 2023

parseplatformorg commented Sep 20, 2023

parseplatformorg commented Nov 16, 2023

mtrezza left a comment •

edited

Loading

Vortec4800 commented Aug 3, 2023 •

edited

Loading

codecov bot commented Aug 14, 2023 •

edited

Loading

mtrezza commented Aug 15, 2023 •

edited

Loading