Laravel quietly ignores Symfony's deprecations: @trigger_error()

[rudiedirkx]

• Laravel Version: 8.73.2
• PHP Version: 7.4.26

Description:

The Symfony ecosystem triggers deprecations like this:

@trigger_error("Some deprecation", E_USER_DEPRECATED);

with the @ to suppress error, to make deprecations opt-in. That's a decent idea, IF the app/framework's error handler CAN opt-in. The first thing Laravel's error handler does is:

if (error_reporting() & $level) {

which means all of Symfony's deprecation triggers are always completely ignored. No opt-in possible.

I think Laravel shouldn't check for error suppression if it's a deprecation, because that's how Symfony does deprecations on purpose. This could work beautifully with isDeprecation() and handleDeprecation(), but it doesn't, and it's too early for the app's Handler to do/catch anything.

Steps To Reproduce:

Trigger a deprecation error, anywhere in your app code, with any Laravel config:

@trigger_error("Some deprecation", E_USER_DEPRECATED);

Without the @ it's reported to my deprecations.log, but with the @ it's quietly ignored. Symfony's standard is to do it with the @, so I feel that should work.

[rudiedirkx]

Some opt-in details on twigphp/Twig#2082

[nunomaduro]

You can log deprecations, from symfony or any other vendor code, by setting up the deprecations log channel: https://laravel.com/docs/8.x/logging#logging-deprecation-warnings.

[rudiedirkx]

No, you can't, because they never ever get past Laravel's error handler:

if (error_reporting() & $level) {

That's the problem.....

[X-Coder264]

I can confirm this issue.

My feature tests use $this->withoutDeprecationHandling(); and I use the Faker library which recently deprecated calling the property name in favour of the method itself -> https://github.com/FakerPHP/Faker/blob/v1.19.0/src/Faker/Generator.php#L942

Since Symfony silences those deprecations the test doesn't fail while my expectation is for it to fail.

So TLDR;

use Illuminate\Foundation\Testing\WithFaker;
use Tests\TestCase;

final class FooTest extends TestCase
{
    use WithFaker;

    public function testFoo(): void
    {
        $this->withoutDeprecationHandling();

        $this->faker->name;
    }
}

Expected result is for the test to fail as Faker emits Since fakerphp/faker 1.14: Accessing property "name" is deprecated, use "name()" instead. deprecation.
Actual result at the moment is that this deprecation is being completely ignored so the test passes.

If I go into vendor and change the Symfony trigger_deprecation method so that it calls trigger_error instead of @trigger_error then the test starts failing (as it should).

[rudiedirkx]

Laravel's handleError should do this IMO:

    if ($this->isDeprecation($level)) {
        return $this->handleDeprecation($message, $file, $line);
    }
    elseif (error_reporting() & $level) {
        throw new ErrorException($message, 0, $level, $file, $line);
    }

ALWAYS handle deprecations. The only reason they wouldn't get past error_reporting() & $level is if it's triggered with @trigger_error, and that's exactly the kinds we DO want. But maybe I'm too deprecation error focused now, I just wanna catch them all. Maybe it needs new config, or a way to override Laravel's handleError, or add more handlers, or whatever.

[driesvints]

@nunomaduro

[rodrigopedra]

Maybe instead of checking for a deprecation error inside the first if

framework/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php

Lines 68 to 77 in afeb58d

	public function handleError($level, $message, $file = '', $line = 0, $context = [])
	{
	if (error_reporting() & $level) {
	if ($this->isDeprecation($level)) {
	return $this->handleDeprecation($message, $file, $line);
	}
	throw new ErrorException($message, 0, $level, $file, $line);
	}
	}

We could check it before:

public function handleError($level, $message, $file = '', $line = 0, $context = [])
{
    if ($this->isDeprecation($level)) {
        return $this->handleDeprecation($message, $file, $line);
    }

    if (error_reporting() & $level) {
        throw new ErrorException($message, 0, $level, $file, $line);
    }
}

As the HandleExceptions@handleDeprecation seems handle it either by logging into the null driver or to a configured driver.

I tested locally with and without suppressing the trigger_error() and it works. Didn't want to send a PR, as this is a very sensitive part of the bootstrap phase and I might be missing something and also I am not sure how to write tests for it.

---

EDIT: Somehow I missed this @rudiedirkx 's comment, which suggests the same:

#40854 (comment)

[nunomaduro]

Fixed.