The current watchdog message in aggregator.fetch.inc, aggregator_aggregator_fetch(), informs the user of an error by referring to the feed's title.

watchdog('aggregator', 'The feed from %site seems to be broken, due to "%error".', array('%site' => $feed->title, '%error' => $result->code . ' ' . $result->error), WATCHDOG_WARNING);

I've found that $feed->title can be null, leading to a PHP error. In addition, $feed->title is not as useful as $feed->url. Given $feed->title, a user needs to dig into configuration to find the feed's url in order to investigate why the feed is returning an error.

I propose the following change:

watchdog('aggregator', 'The feed from %url seems to be broken, due to "%error".', array('%url' => $feed->url, '%error' => $result->code . ' ' . $result->error), WATCHDOG_WARNING);

$feed->url is likely to exist and given the URL in the error message, a developer can more rapidly track down the issue causing the feed access to fail.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jessebeach’s picture

The attached patch replaces $feed->title with $feed-url in the watchdog and dsm messages. This makes the message less likely to themselves create errors and it makes the error messages more useful to developers.

Just to elucidate the issue further, here's an example of the combo of log messages that are currently produced by the existing code:

The feed from seems to be broken, due to "400 Bad Request".; http://www.somesite.org; 1320864166; aggregator; ###.##.###.###; http://www.somesite.org/cron.php?cron_key=#######################-#####################; ; 0;

and this leads to

Notice: Undefined property: stdClass::$title in aggregator_aggregator_fetch() (line 57 of /mnt/www/html/tangle004/docroot/modules/aggregator/aggregator.fetcher.inc).; http://www.somesite.org; 1320864166; php; ###.##.###.###; http://www.somesite.org/cron.php?cron_key=#######################-#####################; ; 0;

jessebeach’s picture

Status: Active » Needs review

setting to needs review.

mstef’s picture

Rerolled #1 to apply to drupal root - 7.x. Needed for drush make.

Status: Needs review » Needs work

The last submitted patch, aggregator_watchdog_feed_error-1337898-3.patch, failed testing.

timhilliard’s picture

Status: Needs work » Needs review

@mikestefff in future please add -D7 to the end or your patch name so that it is ignored by the test bot when the patch does not match the issue version number. i.e. aggregator_watchdog_feed_error-1337898-3-D7.patch

Changing to needs review

jessebeach’s picture

@mikesteff, I rolled the patch with -D7 at the end.

jessebeach’s picture

The old zero-byte patch! This one has content.

cweagans’s picture

Issue tags: +Needs backport to D7

Status: Needs review » Needs work
Issue tags: -Needs backport to D7

The last submitted patch, aggregator_watchdog_feed_error-1337898-1-D7.patch, failed testing.

pwolanin’s picture

Issue summary: View changes
Issue tags: +Needs backport to D7, +Needs reroll
pwolanin’s picture

Issue tags: -Needs reroll
FileSize
2.18 KB

Here's a D8 re-roll.

pwolanin’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: 1337898-11.patch, failed testing.

naxoc’s picture

Status: Needs work » Needs review
FileSize
1.16 KB

Using the url for the error message makes much more sense.

Here is a reroll.

Status: Needs review » Needs work

The last submitted patch, 14: 1337898-12.patch, failed testing.

naxoc’s picture

Status: Needs work » Needs review
FileSize
1.88 KB
742 bytes

Fair enough testbot. Here is a patch with the test updated too.

jhedstrom’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
kerby70’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.89 KB
0 bytes

Reroll attached.

mgifford’s picture

Re-uploading for the bots.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
quietone’s picture

Project: Drupal core » Aggregator
Version: 9.3.x-dev » 1.x-dev
Component: aggregator.module » Code
Assigned: jessebeach » Unassigned

The aggregator module has been removed from Core in 10.0.x-dev and now lives on as a contrib module.
Issues in the Core queue about the aggregator module, like this one, have been moved to the contrib module queue.

larowlan’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Patch needs update for contrib and uses long gone drupal_set_message

dcam’s picture

Version: 1.x-dev » 2.x-dev
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.82 KB

Rerolled #19

dcam’s picture

FileSize
1.37 KB
2.91 KB

The test above failed for two reasons:
* The error message also comes from the DefaultParser, so that had to be updated too. I didn't find any additional instances of that error message.
* I'm assuming the drupal.org webserver redirects http:// traffic to https://. That included the bad URL in the test too. So the URL we gave it didn't match the updated URL after redirection. Changing it to https:/ corrected the problem.

dcam’s picture

FileSize
1.99 KB
3 KB

Array formatting

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me

  • dcam committed 292fb2d6 on 2.x
    Issue #1337898 by dcam, jessebeach, naxoc, kerby70, pwolanin, mgifford,...
dcam’s picture

Version: 2.x-dev » 1.x-dev
Status: Reviewed & tested by the community » Needs review
FileSize
3.65 KB

Just a quick 1.x port

  • dcam committed 943ff96c on 1.x
    Issue #1337898 by dcam, jessebeach, naxoc, kerby70, pwolanin, mgifford,...
dcam’s picture

Project: Aggregator » Drupal core
Version: 1.x-dev » 9.5.x-dev
Component: Code » aggregator.module
Status: Needs review » Active

The patch in #37 was committed to contrib Aggregator. I'm sending this back to the Drupal core queue to be backported and applied to 9.5.x, if anyone would like to do that. Otherwise, this is still tagged as needing a backport to D7.

catch’s picture

Project: Drupal core » Aggregator
Version: 9.5.x-dev » 2.0.2
Component: aggregator.module » Code
Status: Active » Fixed

9.5 is security only now.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.