airbrake / phpbrake

The official Airbrake PHP error notifier
https://airbrake.io
MIT License
48 stars 36 forks source link

According to HHVM, this code is semantically invalid. #88

Closed lexidor closed 5 years ago

lexidor commented 5 years ago

The compiler hh_client from HHVM tells me that I can't run this code.

/vendor/airbrake/phpbrake/src/MonologHandler.php:10:30,71: Unbound name: Monolog\Handler\AbstractProcessingHandler (an object type) (Naming[2049]) /vendor/airbrake/phpbrake/src/MonologHandler.php:22:72,77: Unbound name: Monolog\Logger (an object type) (Naming[2049])

I tend to agree with hh_client here. MonologHandler::class extends \Monolog\Handler\AbstractProcessingHandler, which I am unable to find.

The default argument for parameter 2 Level on MonologHandler::__construct loads a constant from the Monolog\Logger class/interface. I am unable to find this class/interface.

I was looking into porting the library to HHVM, but I believe this isn't right. Feel free to correct me if I am wrong.

vmihailenco commented 5 years ago

It is supposed to use https://github.com/Seldaek/monolog/blob/master/src/Monolog/Handler/AbstractProcessingHandler.php#L22

lexidor commented 5 years ago

:confounded: :confused: I better dropkick composer then. I didn't receive monolog.

composer require airbrake/phpbrake
Using version ^0.6.0 for airbrake/phpbrake
./composer.json has been created
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 7 installs, 0 updates, 0 removals
  - Installing cash/lrucache (1.0.0): Loading from cache
  - Installing guzzlehttp/promises (v1.3.1): Loading from cache
  - Installing ralouphie/getallheaders (2.0.5): Loading from cache
  - Installing psr/http-message (1.0.1): Loading from cache
  - Installing guzzlehttp/psr7 (1.5.2): Loading from cache
  - Installing guzzlehttp/guzzle (6.3.3): Loading from cache
  - Installing airbrake/phpbrake (v0.6.0): Loading from cache
guzzlehttp/guzzle suggests installing psr/log (Required for using the Log middleware)
Writing lock file
Generating autoload files
lexidor commented 5 years ago

Now this becomes a how does composer work question. You are extending class that is in your dev dependencies. Is that allowed in a non-dev folder (such as src)?

    "require-dev": {
        "phpunit/phpunit": "^5.7",
        "squizlabs/php_codesniffer": "^2.9",
        "monolog/monolog": "^1.22"
    }
vmihailenco commented 5 years ago

The intention here is to make monolog an optional dependency because:

So you only need to import MonologHandler.php if you are using monolog.

lexidor commented 5 years ago

Thanks for the clarification. I can safely discard the file without an side effects, provided that Monolog is not being used. Thanks a ton.