watchdog() is used by the custom error handler that is set by Drupal. Watchdog() gets the ip address by calling Drupal::request()->getClientIP(), which requires that the service container is set. This makes all errors really serious, since the container is not set yet.

This could possibly also affect other parts of the system which are called early.

#3 drupal-move-error-handler-1974570-3.patch1.41 KBfabsor
FAILED: [[SimpleTest]]: [MySQL] 54,501 pass(es), 10 fail(s), and 0 exception(s). View


fabsor’s picture

There are two ways I can think of to solve this issue:

1. Set the service container in the BOOTSTRAP_CONFIGURATION phase.
2. Modify watchdog and other possible places to check if the container is set where appropriate.

It seems to me that the first choice makes the most sense.

fabsor’s picture

Title: watchdog() can be called before the service container is set » The service container is used before it's set in the drupal error handler
fabsor’s picture

Status: Active » Needs review
1.41 KB
FAILED: [[SimpleTest]]: [MySQL] 54,501 pass(es), 10 fail(s), and 0 exception(s). View

The minimal effort is to move the error handler code to the boot phase, after initializing the service container. This not ideal, since we probably want to use our error handler as early in the process as possible, but it would be hard to refactor it to work, but I'm setting this to needs review anyway to get some eyes on it =)

watchdog() uses the service container for getting the ip_address(), but it also calls hook_watchdog through module_implements, which requires the service container as well.

Status: Needs review » Needs work

The last submitted patch, drupal-move-error-handler-1974570-3.patch, failed testing.

clemens.tolboom’s picture

clemens.tolboom’s picture

The definition of class Drupal states it's a

However, there are cases, particularly in legacy procedural code, where that is infeasible.

and Drupal::request has even

Note: The use of this wrapper in particular is especially discourged.

Should we patch all calls to Drupal::request() or even calls to Drupal::* by using a try-catch construct?

By using #3 we are putting stuff 'under the carpet' I guess.

(my 2 cents)

ParisLiakos’s picture

the_phi’s picture

Priority: Normal » Critical
catch’s picture

Status: Needs work » Closed (duplicate)