Problem/Motivation

Some times we forget to catch the exceptions generated by the wrap() and wrapMultiple methods. I have come to think that this was a bad pattern since it lead to a poor DX.

API changes

Let's return NULL when the entity cannot be wrapped instead of throwing an exception.

This will require a major version bump since the public interface will change. We will also need to add a change record to it.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

e0ipso created an issue. See original summary.

e0ipso’s picture

First pass, let's see what the testbot thinks.

Status: Needs review » Needs work

The last submitted patch, 2: 3181885-implement-a-graceful--2.patch, failed testing. View results

e0ipso’s picture

Let's see how we can test those PHP errors.

e0ipso’s picture

Status: Needs work » Needs review
FileSize
25.39 KB

Status: Needs review » Needs work

The last submitted patch, 5: 3181885-implement-a-graceful--5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

e0ipso’s picture

e0ipso’s picture

Status: Needs work » Needs review
e0ipso’s picture

Status: Needs review » Fixed
e0ipso’s picture

On second thought, we might want to drop the trigger_error to stop spamming the logs.

There is a legit use case to try to wrap an entity that has no repository.

function physical_media_node_access($node, $op, $account) {
  try {
    $wrapped_node = \Drupal::service(RepositoryManager::class)->wrap($node);
  }
  catch (RepositoryNotFoundException $exception) {
    return AccessResult::neutral();
  }
  return $wrapped_node instanceof AccessibleInterface
    ? $wrapped_node->access($op, $account, TRUE)
    : AccessResult::neutral();
}

In this case we want node types that have no associated wrapper to return access neutral. In 3.x the code should look like:

function physical_media_node_access($node, $op, $account) {
  $wrapped_node = typed_entity_repository_manager()->wrap($node);
  return $wrapped_node instanceof AccessibleInterface
    ? $wrapped_node->access($op, $account, TRUE)
    : AccessResult::neutral();
}
e0ipso’s picture

Version: 2.x-dev » 3.x-dev
Status: Fixed » Needs work
e0ipso’s picture

This drops the trigger_error and beautifies the code.

e0ipso’s picture

Status: Needs work » Needs review
e0ipso’s picture

Status: Needs review » Fixed

Merging and fixing the change record.

  • e0ipso committed c824d8f on 3.x
    Issue #3181885 by e0ipso: Implement a graceful fallback check for when...

Status: Fixed » Closed (fixed)

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