Skip to content

Fixed crash in Redis adapter when storeConfig is not provided#27003

Open
muratcorlu wants to merge 3 commits intoTryGhost:mainfrom
muratcorlu:fix/redis-adapter-retry-strategy-crash
Open

Fixed crash in Redis adapter when storeConfig is not provided#27003
muratcorlu wants to merge 3 commits intoTryGhost:mainfrom
muratcorlu:fix/redis-adapter-retry-strategy-crash

Conversation

@muratcorlu
Copy link
Copy Markdown
Contributor

@muratcorlu muratcorlu commented Mar 27, 2026

When storeConfig was omitted from the Redis cache adapter config, the retryStrategy callback would throw a TypeError whenever ioredis tried to reconnect to Redis, crashing Ghost instead of recovering gracefully. Fixed by using optional chaining on config.storeConfig.

Also fixed missing (config.username) JSDoc definition.

Got some code for us? Awesome 🎊!

Please take a minute to explain the change you're making:

  • Why are you making it?
  • What does it do?
  • Why is this something Ghost users or developers need?

Please check your PR against these items:

  • I've read and followed the Contributor Guide
  • I've explained my change
  • I've written an automated test to prove my change works

We appreciate your contribution! 🙏

muratcorlu and others added 2 commits March 27, 2026 15:00
When storeConfig was omitted from the Redis cache adapter config,
the retryStrategy callback would throw a TypeError whenever ioredis
tried to reconnect to Redis, crashing Ghost instead of recovering
gracefully. Fixed by using optional chaining on config.storeConfig.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 27, 2026

Walkthrough

This change enhances the Redis cache adapter's error handling and documentation. The implementation adds JSDoc support for an optional config.username parameter and updates the retryStrategy function to use optional chaining when accessing config.storeConfig.retryConnectSeconds. This prevents potential TypeErrors when storeConfig is undefined, ensuring the retry strategy defaults to 10 seconds. Accompanying test coverage validates that retryStrategy correctly handles three scenarios: missing storeConfig, missing retryConnectSeconds, and explicitly configured retry intervals.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title accurately and concisely summarizes the main change: fixing a crash in the Redis adapter when storeConfig is not provided, which matches the core issue addressed in the changeset.
Description check ✅ Passed The pull request description clearly explains the crash scenario, the root cause, the fix using optional chaining, and the JSDoc documentation addition, with all changes matching the provided changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@muratcorlu muratcorlu changed the title 🐛 Fixed crash in Redis adapter when storeConfig is not provided Fixed crash in Redis adapter when storeConfig is not provided Mar 27, 2026
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant