Skip to content

Fix Redis driver treating every query as failed - #58

Merged
matthi4s merged 1 commit into
masterfrom
fix-redis-last-error
Oct 29, 2026
Merged

matthi4s merged 1 commit into
masterfrom
fix-redis-last-error

Conversation

@matthi4s

Copy link
Copy Markdown
Member

Since #34 (v5.0.0), RedisQueryException::checkConnection() throws unless Redis::getLastError() returns false. But phpredis returns null when there is no error (checked 5.3.7, 6.0.0 and 6.3.0), and false only when there is no connection at all. So every save(), get() and delete() of a model with a cache time throws an empty RedisQueryException: Redis Query Error: , which makes the Redis cache unusable.

Changes

  • checkConnection() only throws if getLastError() returns a string, and clears the error before throwing. phpredis keeps the last error until clearLastError() is called, so it would otherwise be reported again for every later query.
  • save(), get() and delete() clear the last error before running their command, so an error left by another command on the same connection (e.g. when the connection is shared with other code) isn't reported as a model error.
  • Add RedisQueryExceptionTest (uses a \Redis stub, requires ext-redis) and enable the redis extension in the test workflow so it runs in CI.

Testing

  • Full test suite passes with phpredis 6.3.0 (88 tests); the new test fails on the old code.
  • Checked against a real Redis 7.4 together with MySQL and OpenSearch: cached save/get/delete work again, a real error from the driver's own command (WRONGTYPE) is still thrown, and the next query works normally afterwards.

Models without a cache time are not affected, since the driver returns before connecting.

@matthi4s
matthi4s requested a review from JulianVennen October 29, 2026 12:10
@matthi4s
matthi4s merged commit 3aa8c91 into master Oct 29, 2026
2 checks passed
@JulianVennen
JulianVennen deleted the fix-redis-last-error branch October 29, 2026 12:18
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.

2 participants