Make $errcontext nullable

[gnumoksha]

Hi. In addition to #892 can you make the parameter $errcontext nullable? I'm having a poor time with Whoops.

[Jean85]

Why should we make it nullable? It already has a default value, so it can be used without any issues without passing it through.

[ste93cry]

I don't get what's the problem here. Our signature includes the $errcontext typehinted as an array and Whoops as far as I see does not call the previous error handler (our), and even if it did and the fifth argument was not passed then the default value would be used so everything should be fine

[gnumoksha]

Why should we make it nullable?

• Default value does not allow null
• The parameter is not used in any location
• This modification is simple
• This modification has no side effects
• PHP world is changing to the use of type hinting, but the process is not finished
• PHP itself has problems with types (e.g. ini_set, SoapClient)

[Jean85]

Default value does not allow null

Yes, but it is optional, so not passing it doesn't create any issue.

The parameter is not used in any location

It's not used but it's passed through to the next error handler in the stack. We cannot pass an invalid value

PHP world is changing to the use of type hinting, but the process is not finished

array is not a new type; and it's the correct type to be used, as indicated in the documentation.

[gnumoksha]

I don't get what's the problem here. Our signature includes the $errcontext typehinted as an array and Whoops as far as I see does not call the previous error handler (our), and even if it did and the fifth argument was not passed then the default value would be used so everything should be fine

You are right, maybe the problem I'm facing is not with Whoops.

The error is:

 [Thu Oct 31 17:05:25.189060 2019] [php7:notice] [pid 99] [client 172.18.0.9:51860] PHP Fatal error:  Uncaught TypeError: Argument 5 passed to Sentry\\ErrorHandler::handleError() must be of the type array, null given in /var/www/vendor/sentry/sentry/src/ErrorHandler.php:372
Stack trace:
#0 [internal function]: Sentry\\ErrorHandler->handleError(8192, 'Function Redis:...', 'Unknown', 0, NULL)
#1 [internal function]: Phalcon\\Cache\\Backend\\Redis->save('ecpf31kem32tp7t...', 'tfluxv3#isNewCa...', 86400)
#2 [internal function]: Phalcon\\Session\\Adapter\\Redis->write('ecpf31kem32tp7t...', 'tfluxv3#isNewCa...')
#3 [internal function]: session_write_close()
#4 [internal function]: Phalcon\\Session\\Adapter->__destruct()
#5 {main}
  thrown in /var/www/vendor/sentry/sentry/src/ErrorHandler.php on line 372, referer: http://test.foo.bar/foo-bar-baz

So, my stack includes phalcon, whoops, codeception and Sentry. Since this error only occurs in one of the tests suites and codeception does not use type hinting, the error may be there.

Nevertheless, it enforces my fifth argument and Sentry should add the proposed change in order to be more resilient, as an error handler should be.

[ste93cry]

Default value does not allow null

This is because in the PHP documentation there is no mention that the parameter can be nullable. However, I checked the source code of PHP and effectively during shutdown the symbol table can be null, thus even if not documented correctly this parameter must allow that value

This modification is simple
This modification has no side effects

Indeed, PR is welcome

Since this error only occurs in one of the tests suites and codeception does not use type hinting, the error may be there.

The fact that Codeception is not using type hinting means nothing in this case. The problem is that the $errcontext is holding a value that per-documentation should not be allowed, and the cause of this issue is who is setting it that way (or, in this specific case it's just a documentation issue). Of course, if Codeception used type hinting then it would break in the same way

PHP world is changing to the use of type hinting, but the process is not finished
Sentry should add the proposed change in order to be more resilient, as an error handler should be.

This has nothing to do with the issue. If a piece of code expects the parameters/variables to be of a certain type and someone else using it passes something else then it's up to him to fix the problem because he's using it wrongly and not whom wrote that code. But as I said before, in this specific case the problem is a non-accurate documentation