Page MenuHomePhabricator

Make "bin/aphlict notify" send notifications in-process
Open, LowPublic

Description

See PHI1534. Currently, bin/aphlict notify ... queues a notification task into the daemon queue, but does not actually send it inline. This makes --trace a little confusing, and the success message ("Sent notification.") misleading.

Using PhabricatorWorker::setRunAllTasksInProcess(true); to force the task to execute in-process hits this:

[2019-11-07 14:27:00] EXCEPTION: (PhabricatorWorkerPermanentFailureException) Feed story (with key "6756694889126356052") does not exist or could not be loaded. at [<phabricator>/src/applications/feed/worker/FeedPushWorker.php:15]
arcanist(head=stable, ref.master=cc1ff38843c4, ref.stable=db1959900a64), corgi(head=master, ref.master=72dc914db4d4), instances(head=master, ref.master=7ecf93d378a1), libcore(), phabricator(head=master, ref.master=f5f2a0bc5696, custom=9), phutil(head=master, ref.master=39ed96cd818a), services(head=master, ref.master=c4bd119b358e)
  #0 FeedPushWorker::loadFeedStory() called at [<phabricator>/src/applications/feed/worker/FeedPublisherWorker.php:6]
  #1 PhabricatorWorker::scheduleTask(string, array) called at [<phabricator>/src/applications/feed/PhabricatorFeedStoryPublisher.php:151]
  #2 PhabricatorFeedStoryPublisher::publish() called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:3844]
  #3 PhabricatorApplicationTransactionEditor::publishFeedStory(PhabricatorUser, array, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1513]
  #4 PhabricatorApplicationTransactionEditor::publishTransactions(PhabricatorUser, array) called at [<phabricator>/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php:21]
  #5 PhabricatorApplicationTransactionPublishWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:124]
  #6 PhabricatorWorker::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:163]
  #7 PhabricatorWorker::scheduleTask(string, array, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1462]
  #8 PhabricatorApplicationTransactionEditor::queuePublishing() called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1386]
  #9 PhabricatorApplicationTransactionEditor::applyTransactions(PhabricatorUser, array) called at [<phabricator>/src/applications/aphlict/management/PhabricatorAphlictManagementNotifyWorkflow.php:77]
  #10 PhabricatorAphlictManagementNotifyWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:457]
  #11 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:349]
  #12 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/support/aphlict/server/aphlict_launcher.php:23]

I initially thought this was an issue with transaction visibility, but the notification actually goes through, so I think it's really a problem with the story legitimately not being loadable (presumably because this notification/story is unusual).

Event Timeline

epriestley created this task.

This looks like an edge case that fell out of D19864, which was originally motivated by similar goals (fixing how test notifications work).

I'm fixing it by adding a new test for $story->isVisibleInFeed() before queueing feed-specific publishing steps.

But this doesn't work since isVisibleInFeed() is on PhabricatorFeedStory, not PhabricatorFeedStoryData. We only have a Data and there's no easy way to build a Story out of it.

Since this class tree is effectively obsolete (all Story objects are of class PhabricatorApplicationTransactionFeedStory today, and this seems unlikely to change) the better fix here is probably to clean up a bunch of this old feed code.