Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect PHP ini settings for error reporting #437

Open
sebastianfeldmann opened this issue Oct 18, 2024 · 5 comments
Open

Respect PHP ini settings for error reporting #437

sebastianfeldmann opened this issue Oct 18, 2024 · 5 comments
Assignees
Labels

Comments

@sebastianfeldmann
Copy link
Contributor

phive registers its own error handler

set_error_handler([$this, 'errorHandler']);

that does not respect the PHP ini error_reporting setting.

This is a bit problematic while running builds for new PHP versions.
While phive would most likely run under php 8.4 without any problems but because there are deprecation errors it just crashes.

I think, during execution time deprecations can safely be ignored.

@theseer
Copy link
Member

theseer commented Oct 18, 2024

I do see your point. I'll consider it.

I'd also like to have an issue opened for the deprecation though ;)

@sebastianfeldmann
Copy link
Contributor Author

sebastianfeldmann commented Oct 18, 2024

Sure thing :)

issue #438

@theseer theseer self-assigned this Oct 18, 2024
@sebastianfeldmann
Copy link
Contributor Author

sebastianfeldmann commented Oct 18, 2024

Could be fixed like this.
I can open a PR if you want.

private $migrationService;

    /** @var MigrationService */
    private $migrationService;

    private $errorLevel;

error_reporting(-1);

    private function setupRuntime(): void {
        $this->errorLevel = error_reporting(-1);

public function errorHandler(int $code, string $message, string $file, int $line): bool {

    public function errorHandler(int $code, string $message, string $file, int $line): bool {
        if (!($this->errorLevel & $code)) {
            return false;
        }

@theseer
Copy link
Member

theseer commented Oct 18, 2024

I'm not sure I understand the proposed change. It uses a hardcoded -1 - so it's identical to what I have - isn't it? Maybe it's too late and I'm jumping between too many projects..

@sebastianfeldmann
Copy link
Contributor Author

sebastianfeldmann commented Oct 18, 2024

Flow:

  • // original behavior (keep)
    trigger all errors error_reporting(-1)
  • // proposed change
    use private to store original php ini setting $this->errorLevel = trigger_error(-1)
  • // propose change
    while handling errors use the original php ini setting to make the decision if an exception should be thrown if (!($this->errorLevel & $code)) {

this way you could still do something with deprecations in the future (print as warnings after execution...)
but it would not break the phive execution anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants