Stacking error handlers

[derrabus]

I'd like to register a temporary error handler that decorates my global error handler. I usually do that this way:

$previous = null;
$previous = set_error_handler(static function () use (&$previous): bool {
    // Special error handling here.
    
    // Fall back to the global error handler
    return is_callable($previous) && $previous(...func_get_args());
});

try {
    // Do something
} finally {
    restore_error_handler();
}

If you know a more elegant way to do this, please enlighten me. Psalm however does not like at all what I'm doing here. 🙈

Minimal version:

<?php

$previous = null;
$previous = set_error_handler(static function () use (&$previous): bool {
    return is_callable($previous) && $previous(...func_get_args());
});

https://psalm.dev/r/8d2ca309f8

ERROR: TypeDoesNotContainType - 5:12 - Type null for $previous is never callable

ERROR: InvalidFunctionCall - 5:38 - Cannot treat type never as callable

INFO: UnusedVariable - 4:1 - $previous is never referenced or the value is not used

All three errors are false positives imo.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/8d2ca309f8

<?php

$previous = null;
$previous = set_error_handler(static function () use (&$previous): bool {
    return is_callable($previous) && $previous(...func_get_args());
});

Psalm output (using commit bf57d59):

ERROR: TypeDoesNotContainType - 5:12 - Type null for $previous is never callable

ERROR: InvalidFunctionCall - 5:38 - Cannot treat type never as callable

INFO: UnusedVariable - 4:1 - $previous is never referenced or the value is not used

[weirdan]

Psalm just needs a little help: https://psalm.dev/r/df9f6e0be7

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/df9f6e0be7

<?php
/** @var null|callable */
$previous = null;

$previous = set_error_handler(static function () use (&$previous): bool {
    return is_callable($previous) && $previous(...func_get_args());
});

Psalm output (using commit bf57d59):

No issues!

[derrabus]

Thanks! This workaround is helpful, but then again: should Psalm need this kind of help? Shall we keep the issue open?

[weirdan]

References are intrinsically problematic to analyze (see https://en.wikipedia.org/wiki/Action_at_a_distance_(computer_programming)) , so I would say yes.

You can rewrite this as a class where (hidden) references are (implicitly) typed, so providing this additional information Psalm needs would feel more natural. E.g. something like this: https://psalm.dev/r/d5103d86ae

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/d5103d86ae

<?php

final class ErrorHandler {
    /** @var callable|null */
    private static $previous = null;
    
    public static function run(callable $suppress, callable $callback, mixed ...$args): mixed {
		self::install($suppress);
        try {
            return $callback(...$args);
        } finally {
            restore_error_handler();
        }
    }
    
    private static function install(callable $handler): void {
        self::$previous = set_error_handler(
            function(
                int $errno, 
                string $errstr, 
                string $errfile = 'unknown', 
                int $errline = 0, 
                array $errcontext = []
            ) use ($handler) {
            	return $handler(...func_get_args()) 
                    && is_callable(self::$previous) 
                    && (self::$previous)(...func_get_args());
        	}
        );
    }
}

return ErrorHandler::run(
    fn (mixed $_, string $errstr) => str_contains($errstr, 'ORA-123'),
    query(...),
    "select something",
);

function query(string $_query): void {}

Psalm output (using commit bf57d59):

No issues!