Closed (fixed)
Project:
Aggregator2
Version:
master
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
17 Jul 2006 at 07:24 UTC
Updated:
20 Jul 2006 at 16:31 UTC
Jump to comment: Most recent file
Hello,
I recently had a problem where I was unable to parse a particular feed with aggregator2, and was getting no error message or other form of debugging feedback. It took me about a half an hour to figure out that the domain name lookup in cURL was just timing out.
aggregator2_http_request should check curl_errno() after the call to curl_exec() in order to see if there was an error during the fetching of the RSS feed, and then curl_error() to retrieve the error and pass it on up so it can be displayed to the user. I don't know what the best way to do this is, perhaps have aggregator2_http_request return null on a cURL error and then check the return value of aggregator2_http_request() when it is called from elsewhere. I don't know how to pass the error message up in a nice way though.
Does anyone have any suggestions?
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | agg2_curl_error.patch | 1.16 KB | benwei |
Comments
Comment #1
Christoph C. Cemper commentedIMHO this is not a feature request but a bug... any software should check for errors and act accordingly...
>I don't know how to pass the error message up in a nice way though.
I'd say it's a "watchdog" - message... no user message should be displayed for such fatals
Comment #2
benwei commentedAgreed that this is really a bug.
However, I do think that the error should be reported up to the user. This would have saved me a lot of time. I guess I can just use drupal_set_message for this. This is what the attached patch does (however, you could remove two lines and have it go back to the old behavior plus a watchdog message.)
The problem as I see it is that Drupal is already reporting an error to the user in this case, but it is just a generic message saying 'failed to parse RSS feed' and empty error string. That error message is not really appropriate in this case, as there was no error in parsing (indeed, we never even get to parsing). I think it would be better to have the actual error shown to the user. They need to have some kind of indication that the update failed in any case, so why not give them the reason for the failure?
Comment #3
benwei commentedForgot to change status. Marking as 'code needs work' because I am not sure whether the final decision will be to display the error to the user or just watchdog it.
Comment #4
ahwayakchih commentedThanks for patch.
I've just commited changes to CVS (they are a bit different than Your patch, but still should work ok - please try it and if it doesn't work re-open this issue :).
Comment #5
ahwayakchih commented