Problem/Motivation

As a continuation of this problem that I originally posted under Push Framework #3218315: Gracefully fail on a non-existent notification, it turns out it's actually a DANSE issue (because I tested it without Push Framework being available and it still broke Drush). I've also confirmed that this happens on v2.2.0 and 2.2.dev

The goal is to be able to create and delete an entity, which creates a DANSE event, before cron runs. Right now, doing this causes a cron error.

The reason we need this functionality is because we have an environment where content is moderated. It starts off in a draft state and then is reviewed by moderators and published or deleted. The problem comes in where a moderator might delete an entity before it's notification has been processed by cron and then cron is basically non-functional until we run all the edel commands (drush edel danse_event at minimum). The problem is that there may also be other valid notifications that haven't yet gone out and those get lost when we run the edel commands.

If DANSE could find a way to not break cron when entities are deleted before it runs, that would be awesome.

Steps to reproduce

Get DANSE setup, I'm personally focusing on individual entity subscriptions and nothing else.

  1. Fresh Site setup
  2. Enable DANSE and DANSE_content
  3. Create article and comments, setup DANSE subscriptions for both
  4. Subscribe to comments on article entity
  5. Post a comment
  6. Delete the comment
  7. Run drush cron
  8. Notice that drush cron now has errors as seen below:
[error]  TypeError: Argument 2 passed to Drupal\danse\PluginBase::subscriptionKey() must be of the type string, null given, called in /var/www/html/website.com/web/modules/contrib/danse/modules/content/src/Payload.php on line 68 in Drupal\danse\PluginBase->subscriptionKey() (line 224 of /var/www/html/website.com/web/modules/contrib/danse/src/PluginBase.php) #0 /var/www/html/website.com/web/modules/contrib/danse/modules/content/src/Payload.php(68): Drupal\danse\PluginBase->subscriptionKey()
#1 /var/www/html/website.com/web/modules/contrib/danse/src/PluginBase.php(270): Drupal\danse_content\Payload->getSubscriptionReferences()
#2 /var/www/html/website.com/web/modules/contrib/danse/src/PluginBase.php(159): Drupal\danse\PluginBase->getSubscribers()
#3 /var/www/html/website.com/web/modules/contrib/danse/src/Service.php(259): Drupal\danse\PluginBase->createNotifications()
#4 /var/www/html/website.com/web/modules/contrib/danse/danse.module(20): Drupal\danse\Service->createNotifications()
#5 [internal function]: danse_cron()
#6 /var/www/html/website.com/web/core/lib/Drupal/Core/Extension/ModuleHandler.php(392): call_user_func_array()
#7 /var/www/html/website.com/web/core/lib/Drupal/Core/Cron.php(236): Drupal\Core\Extension\ModuleHandler->invoke()
#8 /var/www/html/website.com/web/core/lib/Drupal/Core/Cron.php(134): Drupal\Core\Cron->invokeCronHandlers()
#9 /var/www/html/website.com/web/core/lib/Drupal/Core/ProxyClass/Cron.php(75): Drupal\Core\Cron->run()
#10 /var/www/html/website.com/vendor/drush/drush/src/Drupal/Commands/core/DrupalCommands.php(76): Drupal\Core\ProxyClass\Cron->run()
#11 [internal function]: Drush\Drupal\Commands\core\DrupalCommands->cron()
#12 /var/www/html/website.com/vendor/consolidation/annotated-command/src/CommandProcessor.php(257): call_user_func_array()
#13 /var/www/html/website.com/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback()
#14 /var/www/html/website.com/vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter()
#15 /var/www/html/website.com/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(302): Consolidation\AnnotatedCommand\CommandProcessor->process()
#16 /var/www/html/website.com/vendor/symfony/console/Command/Command.php(255): Consolidation\AnnotatedCommand\AnnotatedCommand->execute()
#17 /var/www/html/website.com/vendor/symfony/console/Application.php(1005): Symfony\Component\Console\Command\Command->run()
#18 /var/www/html/website.com/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand()
#19 /var/www/html/website.com/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun()
#20 /var/www/html/website.com/vendor/drush/drush/src/Runtime/Runtime.php(118): Symfony\Component\Console\Application->run()
#21 /var/www/html/website.com/vendor/drush/drush/src/Runtime/Runtime.php(48): Drush\Runtime\Runtime->doRun()
#22 /var/www/html/website.com/vendor/drush/drush/drush.php(72): Drush\Runtime\Runtime->run()
#23 /var/www/html/website.com/vendor/drush/drush/drush(4): require('/var/www/html/w...')
#24 {main}.
TypeError: Argument 2 passed to Drupal\danse\PluginBase::subscriptionKey() must be of the type string, null given, called in /var/www/html/website.com/web/modules/contrib/danse/modules/content/src/Payload.php on line 68 in /var/www/html/website.com/web/modules/contrib/danse/src/PluginBase.php on line 224 #0 /var/www/html/website.com/web/modules/contrib/danse/modules/content/src/Payload.php(68): Drupal\danse\PluginBase->subscriptionKey()
#1 /var/www/html/website.com/web/modules/contrib/danse/src/PluginBase.php(270): Drupal\danse_content\Payload->getSubscriptionReferences()
#2 /var/www/html/website.com/web/modules/contrib/danse/src/PluginBase.php(159): Drupal\danse\PluginBase->getSubscribers()
#3 /var/www/html/website.com/web/modules/contrib/danse/src/Service.php(259): Drupal\danse\PluginBase->createNotifications()
#4 /var/www/html/website.com/web/modules/contrib/danse/danse.module(20): Drupal\danse\Service->createNotifications()
#5 [internal function]: danse_cron()
#6 /var/www/html/website.com/web/core/lib/Drupal/Core/Extension/ModuleHandler.php(392): call_user_func_array()
#7 /var/www/html/website.com/web/core/lib/Drupal/Core/Cron.php(236): Drupal\Core\Extension\ModuleHandler->invoke()
#8 /var/www/html/website.com/web/core/lib/Drupal/Core/Cron.php(134): Drupal\Core\Cron->invokeCronHandlers()
#9 /var/www/html/website.com/web/core/lib/Drupal/Core/ProxyClass/Cron.php(75): Drupal\Core\Cron->run()
#10 /var/www/html/website.com/vendor/drush/drush/src/Drupal/Commands/core/DrupalCommands.php(76): Drupal\Core\ProxyClass\Cron->run()
#11 [internal function]: Drush\Drupal\Commands\core\DrupalCommands->cron()
#12 /var/www/html/website.com/vendor/consolidation/annotated-command/src/CommandProcessor.php(257): call_user_func_array()
#13 /var/www/html/website.com/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback()
#14 /var/www/html/website.com/vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter()
#15 /var/www/html/website.com/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(302): Consolidation\AnnotatedCommand\CommandProcessor->process()
#16 /var/www/html/website.com/vendor/symfony/console/Command/Command.php(255): Consolidation\AnnotatedCommand\AnnotatedCommand->execute()
#17 /var/www/html/website.com/vendor/symfony/console/Application.php(1005): Symfony\Component\Console\Command\Command->run()
#18 /var/www/html/website.com/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand()
#19 /var/www/html/website.com/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun()
#20 /var/www/html/website.com/vendor/drush/drush/src/Runtime/Runtime.php(118): Symfony\Component\Console\Application->run()
#21 /var/www/html/website.com/vendor/drush/drush/src/Runtime/Runtime.php(48): Drush\Runtime\Runtime->doRun()
#22 /var/www/html/website.com/vendor/drush/drush/drush.php(72): Drush\Runtime\Runtime->run()
#23 /var/www/html/website.com/vendor/drush/drush/drush(4): require('/var/www/html/n...')
#24 {main}
TypeError: Argument 2 passed to Drupal\danse\PluginBase::subscriptionKey() must be of the type string, null given, called in /var/www/html/website.com/web/modules/contrib/danse/modules/content/src/Payload.php on line 68 in Drupal\danse\PluginBase->subscriptionKey() (line 224 of /var/www/html/website.com/web/modules/contrib/danse/src/PluginBase.php).
 [warning] Drush command terminated abnormally.

Comments

rex.barkdoll created an issue. See original summary.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

That's an interesting one - and now that you describe it with the delete context I know exactly why we never ran into this ourselves: when using workflows, CMS, audit or any other related tool, we never ever delete anything. There are so many situations where a deleted entity can and will cause trouble - not only for DANSE.

Our thinking is this: if there ever was a reason to create an entity - and even if it was a false reason - it is important to keep trace of that entity. And if it really should no longer be there, make it invisible to the users - but keep it in the system. That's the only way to keep knowing that there had been an entity that is now no longer available (from the user perspective). So achieve that, at least make the entity unpublished and if you use workflow, then define an extra state which represents the delete state of entities. And while there is no transition out of that delete state, it will just be the same as if it were deleted in the first place. But the system has still access and won't fail, if such an entity is referenced somewhere else - like DANSE audit or notifications.

---

Regardless, you're right that DANSE should not fail in such situations. But that might be a bigger task, because DANSE accesses entities at so many different places, that we have to review the code carefully and everywhere.

rex.barkdoll’s picture

rex.barkdoll commented

Ah, very interesting. I hadn't thought about using that kind of workflow. I'm going to have to think about how to alter our system to work like that because it's how we should be operating.

That said, I think it's still important to ask for this feature because I'm sure other people are going to take the same approach I have and run into the same issue and it should find a way not to break things.

Thanks for looking into this and being open to finding a solution. I appreciate you showing me how you think about the problem and how to work around it.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

I'm currently thinking that we need to catch all the places where an entity doesn't exist anymore, because it got deleted, and then create (not save) a temporary entity of the same type and return that for further processing. That'll mean that a notification might mention "Deleted node" instead of the real node title which we can't determine anymore.

rex.barkdoll’s picture

rex.barkdoll commented

I think I'm going to lean into whatever your intuition says is the best way for handling deleted content. Since I don't have a deep knowledge of how DANSE identifies and tracks content, I don't think I'm in a good place to recommend what should be done.

What I would say is that DANSE shouldn't be forwarding deleted content out to Push Framework (or similar systems) for distribution. If it detects that content has been deleted, I think that it's totally fine for it to make temporary content and note that it's deleted in its log so that it doesn't break anything. I would just make sure the temporary content would then force a silent/no-send state so that users aren't getting linked to non-existent content.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

Here is another thought: we could hook into the entity delete process and check, if any event or notification exist, that link to that to be deleted entity. And if so, delete those events and notifications as well. That would be much easier to implement and we would not have to deal with lots of exceptions in the code all over the place. Of course, we then loose track of notifications that existed in the past but now longer do so. But hey, if the content is gone, the associated event and notification is useless anyways, isn't it?

rex.barkdoll’s picture

rex.barkdoll commented

That sounds like a good solution to me, definitely much easier to implement. I'm not using DANSE for auditing, I use other modules for that, so I don't need it to have a record of everything regardless of its existence.

Playing the devil's advocate: Since "Audit" is in the name, I think it might still be good to have some kind of silent record show up in DANSE's event log, acknowledging that a deleted record existed, but wasn't pushed out because it was deleted.

If you're not worried about that or want to implement it later - if people request it - I don't have an issue with that. Just food for thought.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

That's what I thought, too. Instead of deleting the event and notifications, we should do this:

  • identify all events and notifications linked to a deleted entity
  • replace all the references to that to be deleted entity with something that can't break but tell that the entity got deleted
  • for all not yet delivered notifications, set them to "delivered" so that no further delivery will happen in the future
rex.barkdoll’s picture

rex.barkdoll commented

Looks great! I think that will work for everyone :)

  • jurgenhaas committed ed6b208 on 2.2.x 
    Issue #3231348 by jurgenhaas: Fail gracefully when content is deleted...
baluertl’s picture

baluertl
he/him
Primary language Hungarian
Location Budapest 🇪🇺
commented
Issue summary: View changes
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented
Status: Active » Fixed

Closing this on as #10 should resolve the issue. Please reopen, if that's not the case.

Status: Fixed » Closed (fixed)

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