Unify Database/Log::findCaller and Utility/Error::getLastCaller

Uhm... it looks like there's some overlap between how Drupal\Core\Utility\Error is determining the caller, and a similar functionality in Drupal\Core\Database\Log::findCaller. Wondering is there's a chance to unify that.

Working on #3.

Result array for debug_backtrace() & $e->getTrace() are the same so makes sense to unify

This would be the idea, taking big portions from #2867788: Log::findCaller fails to report the correct caller function with non-core drivers.. If we agree this is the right direction, I can also take more testing from there.

This should fix one of the fails.

Actually both :). Ready for review.

+++ b/core/lib/Drupal/Core/Utility/Error.php
@@ -75,6 +70,33 @@ public static function decodeException($exception) {
+    // @todo in Drupal 9.0, remove the check for deprecated 'db_' functions.

like this? ;)

just a few lines above...

1. +++ b/core/lib/Drupal/Core/Database/Database.php
   @@ -503,7 +503,7 @@ public static function getConnectionInfoAsUrl($key = 'default') {
   -  protected static function getDatabaseDriverNamespace(array $connection_info) {
   +  public static function getDatabaseDriverNamespace(array $connection_info) {
        if (isset($connection_info['namespace'])) {
          return $connection_info['namespace'];
        }
   

   May I ask whether its we need a method for this and not just get the namespace out of $connection_info itself.

2. +++ b/core/lib/Drupal/Core/Database/Log.php
   @@ -2,6 +2,8 @@
    
    namespace Drupal\Core\Database;
    
   +use Drupal\Core\Utility\Error;
   +
    /**
   

   Looking through all Drupal\Core\Database code there is no reference to anything outside of its own namespace. I am wondering whether that's a sign that we should move this error helper methods into a component instead.

3. +++ b/core/lib/Drupal/Core/Utility/Error.php
   @@ -75,6 +70,33 @@ public static function decodeException($exception) {
   +  public static function removeDatabaseEntries(array &$backtrace, $driver_namespace) {
   +    // Starting from the very last entry in the backtrace (which is the first
   +    // call in the request), find the offset of the first function call that
   +    // can be identified as a call to a method/function in the database layer.
   +    // @todo in Drupal 9.0, remove the check for deprecated 'db_' functions.
   +    for ($offset = count($backtrace) - 1; $offset >= 0; $offset--) {
   +      $class = isset($backtrace[$offset]['class']) ? $backtrace[$offset]['class'] : '';
   +      $function = isset($backtrace[$offset]['function']) ? $backtrace[$offset]['function'] : '';
   +      if (
   +        strpos($class, 'Drupal\Core\Database', 0) === 0 ||
   +        strpos($class, $driver_namespace, 0) === 0 ||
   +        strpos($function, 'db_', 0) === 0
   +      ) {
   +        break;
   +      }
   +    }
   +    $backtrace = array_slice($backtrace, $offset);
   +  }
   

   Could this be some sort of generic function instead which allows you to pass along a callable for the filtering? Like backtrace_filter or so?

> Looking through all Drupal\Core\Database code there is no reference to anything outside of its own namespace. I am wondering whether that's a sign that we should move this error helper methods into a component instead.

Yes, the goal with DBTNG was always to keep dependencies to an absolute minimum as the idea back then was to separate this into a standalone library at some that Drupal would just use.

That never fully happened but I think we should really think before adding more dependencies to that component.

#14:

1. 'namespace' may not be set if you have a settings.php still on D7, hence we fall back

  protected static function getDatabaseDriverNamespace(array $connection_info) {
    if (isset($connection_info['namespace'])) {
      return $connection_info['namespace'];
    }
    // Fallback for Drupal 7 settings.php.
    return 'Drupal\\Core\\Database\\Driver\\' . $connection_info['driver'];
  }

if that wasn't the case, then yes that method would be redundant. An idea could be to fill in 'namespace' it if missing in settings.php, at each request. But that will be unnecessary most of the time and then just more overhead.

2. I have no particular argument here. Whatever.

3. Yes, but sitting where then?

So, #2867788: Log::findCaller fails to report the correct caller function with non-core drivers. has now landed; the major concern here seems to be to have Log depend on Error. Let's see if we can do the other way around. Working on it.

Simpler now.

This needs #3112476: Always set $info['namespace'] on database connection info, to ensure that the namespace is available from the connection info, though.

Created the CR and updated the IS.

Fixed and commented in MR

This is a nice little fix.
All code changes look good to me.
There is a CR for the API-addition.
For me it is RTBC.

Nice clean-up!

Committed 981d78c and pushed to 9.2.x. Thanks!