-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Description
Describe the bug
Found will using the new event simplepie_after_init (#7007)
When processing the event, if the request to the feed fails $simplePie->status_code remains at 0 even when the logs show something like:
FreshRSS SimplePie GET 429 http://localhost:4985/custom
[admin] [Sat, 30 Nov 2024 11:59:17 -0500] [warning] --- cURL error 22: The requested URL returned error: 429 [http://localhost:4985/custom]
To Reproduce
- Create an extension which registers to the hook
simplepie_after_initand prints or does something with$simplePie->status_code - Update or add a feed which returns an http error (I just ran a local server to force a 429)
- See the logs of simplepie receiving the error code, while the extension receives
0
Expected behavior
SimplePie should have the http code the request returned.
FreshRSS version
1.25.0-dev
Environment information
- Database version: SQLite
- PHP version: 8.2.24
- Installation type: Docker image source ( https://github.com/FreshRSS/FreshRSS/blob/edge/Docker/Dockerfile )
- Docker compose https://github.com/FreshRSS/FreshRSS/blob/edge/Docker/freshrss/docker-compose.yml
With these changes to run the checked out code
build:
# Pick #latest (stable release) or #edge (rolling release) or a specific release like #1.21.0
context: ../../
dockerfile: Docker/Dockerfile
volumes:
- ../..:/var/www/freshrss
- Web server type: Apache
- Device:
- OS: Ubuntu 24.04
- Browser: Firefox 133
Additional context
Seems the issue is a combination of missing parts
In this section the new exception doesn't have $file->get_status_code()
| throw new HttpException($file->error); |
Here $th->getCode() should be used instead of a hard-coded 0
FreshRSS/lib/simplepie/simplepie/src/SimplePie.php
Lines 1998 to 2002 in a41aadf
| $file = $this->get_http_client()->request(Client::METHOD_GET, $this->feed_url, $headers); | |
| $this->status_code = $file->get_status_code(); | |
| } catch (HttpException $th) { | |
| $this->check_modified = false; | |
| $this->status_code = 0; |
Similarly here, in the catch block the code should be taken from the exception
FreshRSS/lib/simplepie/simplepie/src/SimplePie.php
Lines 2092 to 2101 in a41aadf
| $file = $this->get_http_client()->request(Client::METHOD_GET, $this->feed_url, $headers); | |
| } catch (HttpException $th) { | |
| // If the file connection has an error, set SimplePie::error to that and quit | |
| $this->error = $th->getMessage(); | |
| return !empty($this->data); | |
| } | |
| } | |
| } | |
| $this->status_code = $file->get_status_code(); |
I'm still looking into how actually the property $status_code of File is populated and where is being used to know if there are more cases which might not fill in the value of the response.