[Messenger] silence PHP warnings issued by `Redis::connect()` #58818

xabbuh · 2024-11-09T10:55:29Z

Q	A
Branch?	5.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Issues
License	MIT

Redis::connect() may issue a warning when the host cannot be resolved (see https://github.com/symfony/symfony/actions/runs/11755592846/job/32750804715?pr=58370#step:8:2535)

stof · 2024-11-12T14:45:17Z

how does error handling work for the connection then ? Are we still reporting failures properly ?

chalasr · 2024-11-12T22:02:36Z

how does error handling work for the connection then ? Are we still reporting failures properly ?

Good point, proper error handling is missing for connect(). We need something similar to what the redis cache adapter does.

xabbuh · 2024-11-12T22:11:42Z

you are right, I have updated the class with the same approach that we use in the Redis adapter as pointed out by @chalasr

src/Symfony/Component/Messenger/Bridge/Redis/Transport/Connection.php

nicolas-grekas · 2024-11-13T13:30:34Z

src/Symfony/Component/Messenger/Bridge/Redis/Transport/Connection.php

+        }
+
+        if (!$isConnected) {
+            throw new InvalidArgumentException(sprintf('Redis connection failed: %s', preg_match('/^Redis::p?connect\(\): (.*)/', $error ?? $redis->getLastError() ?? '', $matches) ? \sprintf(' (%s)', $matches[1]) : ''));


Suggested change

throw new InvalidArgumentException(sprintf('Redis connection failed: %s', preg_match('/^Redis::p?connect: (.*)/', $error ?? $redis->getLastError() ?? '', $matches) ? \sprintf(' (%s)', $matches[1]) : ''));

throw new InvalidArgumentException('Redis connection failed: '.(preg_match('/^Redis::p?connect: (.*)/', $error ?? $redis->getLastError() ?? '', $matches) ? \sprintf(' (%s)', $matches[1]) : ''));

nicolas-grekas · 2024-11-13T14:11:31Z

Thank you @xabbuh.

…ction issues (xabbuh) This PR was merged into the 6.4 branch. Discussion ---------- [Cache] silence warnings issued by Redis Sentinel on connection issues | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | | License | MIT related to #58365 and #58818 see https://github.com/symfony/symfony/actions/runs/11936530431/job/33270392688#step:9:607 Commits ------- 97bdf94 silence warnings issued by Redis Sentinel on connection issues

carsonbot added Status: Needs Review Bug Messenger labels Nov 9, 2024

carsonbot added this to the 5.4 milestone Nov 9, 2024

chalasr approved these changes Nov 9, 2024

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Nov 9, 2024

alexandre-daubois approved these changes Nov 12, 2024

View reviewed changes

xabbuh force-pushed the redis-connect-silence-warnings branch from 3af3822 to 38fd27e Compare November 12, 2024 22:11

xabbuh force-pushed the redis-connect-silence-warnings branch from 38fd27e to 6613d26 Compare November 13, 2024 07:23

OskarStark changed the title ~~[Messenger] silence PHP warnings issued by Redis::connect()~~ [Messenger] silence PHP warnings issued by Redis::connect() Nov 13, 2024

xabbuh force-pushed the redis-connect-silence-warnings branch from 6613d26 to 0769891 Compare November 13, 2024 08:51

chalasr reviewed Nov 13, 2024

View reviewed changes

src/Symfony/Component/Messenger/Bridge/Redis/Transport/Connection.php Outdated Show resolved Hide resolved

xabbuh force-pushed the redis-connect-silence-warnings branch from 0769891 to 98ce163 Compare November 13, 2024 10:00

chalasr approved these changes Nov 13, 2024

View reviewed changes

nicolas-grekas reviewed Nov 13, 2024

View reviewed changes

silence PHP warnings issued by Redis::connect()

8ca27e1

xabbuh force-pushed the redis-connect-silence-warnings branch from 98ce163 to 8ca27e1 Compare November 13, 2024 13:58

nicolas-grekas merged commit fde6ae9 into symfony:5.4 Nov 13, 2024
8 of 12 checks passed

xabbuh deleted the redis-connect-silence-warnings branch November 13, 2024 14:16

xabbuh mentioned this pull request Nov 20, 2024

[Cache] silence warnings issued by Redis Sentinel on connection issues #58952

Merged

This was referenced Nov 27, 2024

Release v5.4.48 #59016

Merged

Release v6.4.16 #59017

Merged

Release v7.1.9 #59018

Closed

Release v7.1.9 #59019

Merged

fabpot mentioned this pull request Nov 29, 2024

Release v7.2.0 #59032

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Messenger] silence PHP warnings issued by `Redis::connect()` #58818

[Messenger] silence PHP warnings issued by `Redis::connect()` #58818

	throw new InvalidArgumentException(sprintf('Redis connection failed: %s', preg_match('/^Redis::p?connect\(\): (.*)/', $error ?? $redis->getLastError() ?? '', $matches) ? \sprintf(' (%s)', $matches[1]) : ''));
	throw new InvalidArgumentException('Redis connection failed: '.(preg_match('/^Redis::p?connect\(\): (.*)/', $error ?? $redis->getLastError() ?? '', $matches) ? \sprintf(' (%s)', $matches[1]) : ''));

[Messenger] silence PHP warnings issued by Redis::connect() #58818

[Messenger] silence PHP warnings issued by Redis::connect() #58818

Conversation

Choose a reason for hiding this comment

[Messenger] silence PHP warnings issued by `Redis::connect()` #58818

[Messenger] silence PHP warnings issued by `Redis::connect()` #58818