_drupal_log_error() passes NULL to ThemeManager::setActiveTheme(), violating its typehint

I experienced the same problem with the latest build and this resolved it so that I saw the standard error message again.

Confirming that this patch allowed the standard error message to show.

I've been experiencing this error for more than a week while developing D8 code, and it's frustrating because whenever I make a mistake this error is thrown from error.inc, hiding the actual problem. As an example, I once added a field in my db_insert() which didn't exist in the table. Whoops, but the error was this "ActiveTheme" error which wasn't helpful at all. Since I had made many code additions, it took me a good while to figure out which one caused the error, and what the error realy was.

I tested the patch and yes, I can now see the original error, not the bogus "ActiveTheme" error. This will save me a LOT of time. Thanks.

The obvious question of course is if we can test this and how. I really don't know why this doesn't show up in our existing tests.

Indeed. Let's put our thinking caps on here. Does anyone here have more info on how you've triggered this in the past?

Looks like we only have tests for exceptions but not php errors, which are, for some reason running through a different code path.

Exceptions are DefaultExceptionSubscriber, errors go through _drupal_log_error(), only the second one calls the theme stuff.

Another way I triggered it is when porting a hook_theme() from D7 to D8. D7 defaults to 'function' while D8 defaults to 'template'. So those cases where I had used a theme function in D7 without a 'function' key ended up causing this error in D8, but of course only at the point where Drupal tried to load what it assumed was a template.

I don't think that what I describe here or above in #3 are PHP errors, although a PHP error may eventually result somewhere down the line. Here, it's just a missing template (I would think that's already tested for somewhere?). In #3, it boils down to an error in the SQL statement which is eventually constructed from my bad instructions.

Let's write indeed a test for it.

Given that this happens for all errors and not just some rare special case, I think this is at least major.

Test looks good to me, RTBC if bot agrees.

Things aren't that easy always.

Ok, now testbot is happy :)

Testbot is not happy

dawehner is stupid.

+++ b/core/modules/system/src/Tests/System/ErrorHandlerTest.php
@@ -46,6 +46,11 @@ function testErrorHandler() {
+      '!message' => 'Argument 1 passed to Drupal\error_test\Controller\ErrorTestController::Drupal\error_test\Controller\{closure}() must be of the type array, string given, called in /Users/dawehner/www/d8/core/modules/system/tests/modules/error_test/src/Controller/ErrorTestController.php on line 66 and defined',

I don't understand how this works? How can we have files at /Users/dawehner/www/? Something feels wrong.

Ha, that was fun :)

Here is a test only patch.

This time, less empty.

Hello,

Thanks you very much. I have applied the patch from comment 20 and it works perfectly.

Works for me. Back to RTBC

Nice, that looks better.

I guess a partial patch of the error message would have been OK too, because this will break if the line number changes, but that won't happen often (ever?), as it is inside a test controller that won't change much.

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 8b98854 and pushed to 8.0.x. Thanks!