Problem/Motivation

I found this when saving a node with an invalid uri in a link field:

The website encountered an unexpected error. Try again later.

Drupal\Core\Entity\EntityStorageException: The URI 'www.example.com' is invalid. You must use a valid URI scheme. in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 817 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

Drupal\link\Plugin\Field\FieldType\LinkItem->getUrl() (Line: 166)
Drupal\link\Plugin\Field\FieldType\LinkItem->isExternal() (Line: 25)
Drupal\entity_usage\Plugin\EntityUsage\Track\Link->getTargetEntities() (Line: 175)
Drupal\entity_usage\EntityUsageTrackBase->trackOnEntityCreation() (Line: 202)
Drupal\entity_usage\EntityUsageTrackBase->trackOnEntityUpdate() (Line: 101)
Drupal\entity_usage\EntityUpdateManager->trackUpdateOnEdition() (Line: 44)
entity_usage_entity_update()

Steps to reproduce

I'm dealing with legacy migrated data so normally the field validation would stop the data from getting into this state.
However for a bit more resilience a try catch could be added.

Proposed resolution

Add try / catch / log approach around calls to \Drupal\entity_usage\EntityUsageTrackInterface::getTargetEntities() so that any exceptions arising for entity track plugins are logged and do not prevent saving.

CommentFileSizeAuthor
#3 entity_usage-3503961-2.patch2.73 KBdahousecat
#2 entity_usage-3503961-1.patch2.22 KBdahousecat

Issue fork entity_usage-3503961

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dahousecat created an issue. See original summary.

dahousecat’s picture

dahousecat commented
StatusFileSize
new entity_usage-3503961-1.patch2.22 KB
dahousecat’s picture

dahousecat commented
StatusFileSize
new entity_usage-3503961-2.patch2.73 KB

Huh, actually the exception was thrown via trackUpdateOnEdition, not trackUpdateOnDeletion.
Updated patch to try catch that method too.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs work

Nice find... the problem here is that \Drupal\entity_usage\Plugin\EntityUsage\Track\Link::getTargetEntities should not throw the exception. We should put the. try catch in there. Also I don't think we should be logging here either. I think we should just return an empty array and move on. That means other fields with entities will still be tracked for this entity.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Or perhaps even better would be to put the try catch inside field loop in \Drupal\entity_usage\EntityUsageTrackBase::trackOnEntityUpdate() and \Drupal\entity_usage\EntityUsageTrackBase::trackOnEntityCreation()... it's tricky - that'd be more robust but potentially more likely to catch something you would want to log - so if we do that we should log as well.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I pushed an MR that starts on #5 - just need to add logging service in a BC compat way...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Added logging...

alexpott opened merge request !112

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: trackUpdateOnDeletion throws exception but is never caught » Entity tracking can cause exceptions and prevent entities being saved
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
Issue tags: +Needs tests

Could do with adding an automated test here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: -Needs tests

I think this is ready now. We log any exceptions thrown by track and have test coverage that the exceptions are logged.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes

marcoscano made their first commit to this issue’s fork.

  • marcoscano committed 07472c03 on 8.x-2.x authored by alexpott 
    Issue #3503961 by alexpott, dahousecat, marcoscano: Entity tracking can...
marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented
Status: Needs review » Fixed

Thanks for jumping in and for the generic solution in the base class! 👍

Status: Fixed » Closed (fixed)

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