Catch ValueError coming from favicon lib in case of invalid redirects #2054
Conversation
Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
| try { | ||
| return $this->faviconFactory->get($base_url); | ||
| } catch (\Error $e) { | ||
| //Actual error should be ValueError but that is only implemented in PHP 8+ |
There was a problem hiding this comment.
If it's only in PHP 8+, how can the factory use it? Aren't they using their own ValueError? Because otherwise it can't throw it either.
There was a problem hiding this comment.
Yea I think it's not their own ValueError, the lib doesn't define any Exceptions if I'm not wrong instead this error from the issue came from PHP itself. Which I guess depends on your PHP version.
The docs for the function that is throwing the error did not inlcude further information.
This should catch the error in php 8+ which the report was also based on and below that worst case the error is not caught because they used something else..
There was a problem hiding this comment.
Can we reproduce it somehow so I can trace and update it in the lib?
There was a problem hiding this comment.
I don't know, the linked issue did have some hints on how it probably happens but for that you would need to miss misconfigure a Webserver on purpose, with an invalid redirect for the favicon.
There was a problem hiding this comment.
It would be easier if we just find the feed where this happens. If we don't make this library more resiliant to errors we'll keep running into this.
There was a problem hiding this comment.
True but the reporter was apparently not able to name the affected feed. The feed that was mentioned there did not cause that error.
Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
|
Can we merge this? #2082 is about the same issue but also provided no feed url, since the exception doesn't contain that. |
|
I'm really worried to merge this since it essentially hides a fundamental problem in the library. To me that doesn't seem like a fix that will make our lives better. |
|
OK how can we identify then the URLs that trigger this? I have never seen this problem in my feeds and non-user so far provided a feed with this issue. |
No description provided.