Skip to content

Check for null before logging download status#3561

Open
bmschwa wants to merge 3 commits intonextcloud:masterfrom
bmschwa:master
Open

Check for null before logging download status#3561
bmschwa wants to merge 3 commits intonextcloud:masterfrom
bmschwa:master

Conversation

@bmschwa
Copy link

@bmschwa bmschwa commented Feb 10, 2026

Summary

A very modest check to ensure no crashing on nulls.

Checklist

Copy link
Contributor

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix: set fallback value for missing error message

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents news:updater:update-feed from throwing a TypeError when a feed update fails but getLastUpdateError() returns null, by falling back to a generated error message.

Changes:

  • Capture getLastUpdateError() into a variable and provide a fallback message when it is null.
  • Ensure OutputInterface::writeln() always receives a non-null string in the update-error path.

Comment on lines 68 to +72
if ($updated_feed->getUpdateErrorCount() !== 0) {
$output->writeln($updated_feed->getLastUpdateError());
$message = $updated_feed->getLastUpdateError();
$message ??= 'Could not update feed with id ' . $feedId . ' for user ' . $userId;

$output->writeln($message);
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add/extend unit coverage for the new null-fallback path: when getUpdateErrorCount() != 0 and getLastUpdateError() returns null, this command should write the fallback message and return 255. There is already UpdateFeedTest::testValidFeedError covering the non-null case, but no test currently asserts the new behavior, so regressions could slip in unnoticed.

Copilot generated this review using guidance from repository custom instructions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants