Remove DependencySerializationTrait from JSON API exceptions

These were created in https://git.drupalcode.org/project/jsonapi/-/commit/cdbd063974f645fefad7... - gonna ping Wim.

Hi Dave,

Removed DependencySerializationTrait from the exceptions EntityAccessDeniedHttpException and UnprocessableHttpEntityException.

Adding a patch for the same.

I've reviewed the patch and the linked issues and commit. I don't see any objections why this would result in issues. Setting RTBC, thanks!

Looks like this is redundant, nice find.

Committed 06e7f02 and pushed to 9.3.x. Thanks!

This has caused a regression on a client project of mine. JsonAPI access denied requests are failing due to

Exception: LogicException: The database connection is not serializable. This probably means you are serializing an object that has an indirect reference to the database connection. Adjust your code so that is not necessary. Alternatively, look at DependencySerializationTrait as a temporary solution.
Drupal\Core\Database\Connection->__sleep()() (Line: 1993)

This happens because \Drupal\jsonapi\JsonApiResource\ErrorCollection contains exceptions and exceptions have traces and traces have arguments and these can be anything. And if one of these contains a reference to a database we’re done for.

The super super amusing thing is that the only part of the code from DependencySerializationTrait that is necessary is

  /**
   * {@inheritdoc}
   */
  public function __sleep() {
    $vars = get_object_vars($this);
    return array_keys($vars);
  }

because that happens to stop the trace being serialised as the trace is not returned in get_object_vars($this).

This serialisation occurs because \Drupal\jsonapi\EventSubscriber\DefaultExceptionSubscriber::setEventResponse adds all this to the response object which is serialised as part of the page cache.

I think we should revert this change and then turn this into one that adds a test for serializing an exception with a db connection in the trace as part of a cacheable JSON api response.

Here's a patch to prove the revert doesn't break anything.

Ok, that was unexpected. Nice one.

Committed/pushed the revert to 9.4.x and 9.3.x.

Restoring this back to old metadata for now.

In order to get this moving forward again we need a few things:

1. Create a test that tests the regression regarding serializing a database connection.
2. Define what we need to serialize for an ErrorCollection so we have a clear plan.
3. Create a new issue (or change the IS) with the new plan on removing trait if we still want this.

@bbrala we will be able to remove the DependencySerializationTrait.

One option is to add ExceptionSerializationTrait which does:

  /**
   * {@inheritdoc}
   */
  public function __sleep() {
    $vars = get_object_vars($this);
    return array_keys($vars);
  }

as this prevents the trace being serialised which is what is causing the problems.

But I think in an ideal world we wouldn't be serialising the exceptions at all. It feels wrong.