diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 85f6ec19..d3d5af4f 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -106,6 +106,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-version }} + coverage: none - run: composer validate diff --git a/.github/workflows/unstable-version-tests.yml b/.github/workflows/unstable-version-tests.yml index 459b2813..d86583fd 100644 --- a/.github/workflows/unstable-version-tests.yml +++ b/.github/workflows/unstable-version-tests.yml @@ -20,6 +20,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-version }} + coverage: none - run: composer validate diff --git a/CHANGELOG.md b/CHANGELOG.md index b68830e1..6e68db50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Changelog ========= +## TBD + +### Bug Fixes + +* Fix a possible crash in the OOM bootstrapper with an incomplete container + [#442](https://github.com/bugsnag/bugsnag-laravel/pull/442) + ## 2.22.0 (2021-02-10) ### Enhancements diff --git a/phpunit.xml.dist b/phpunit.xml.dist index f42de868..e960dc8d 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -18,6 +18,7 @@ <testsuites> <testsuite name="Bugsnag Laravel Test Suite"> <directory suffix="Test.php">./tests</directory> + <directory suffix=".phpt">./tests/phpt</directory> </testsuite> </testsuites> diff --git a/src/OomBootstrapper.php b/src/OomBootstrapper.php index 1e7826db..6c8dd1c8 100644 --- a/src/OomBootstrapper.php +++ b/src/OomBootstrapper.php @@ -39,15 +39,26 @@ public function bootstrap() $lastError = error_get_last(); - // If this is an OOM and memory increase is enabled, bump the memory - // limit so we can report it - if ($lastError !== null - && app('bugsnag')->getMemoryLimitIncrease() !== null - && preg_match($this->oomRegex, $lastError['message'], $matches) === 1 - ) { + if (!$lastError) { + return; + } + + $isOom = preg_match($this->oomRegex, $lastError['message'], $matches) === 1; + + if (!$isOom) { + return; + } + + /** @var \Bugsnag\Client|null $client */ + $client = app('bugsnag'); + + // If the client exists and memory increase is enabled, bump the + // memory limit so we can report it. The client can be missing when + // the container isn't complete, e.g. when unit tests are running + if ($client && $client->getMemoryLimitIncrease() !== null) { $currentMemoryLimit = (int) $matches[1]; - ini_set('memory_limit', $currentMemoryLimit + app('bugsnag')->getMemoryLimitIncrease()); + ini_set('memory_limit', $currentMemoryLimit + $client->getMemoryLimitIncrease()); } }); } diff --git a/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_missing.phpt b/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_missing.phpt new file mode 100644 index 00000000..e4a7c8e1 --- /dev/null +++ b/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_missing.phpt @@ -0,0 +1,47 @@ +--TEST-- +The OomBootstrapper should not crash when bugsnag is not in the DI container +--FILE-- +<?php + +use Bugsnag\BugsnagLaravel\OomBootstrapper; + +function app($alias = null) { + echo "'app' was called!\n"; + + if ($alias === 'bugsnag') { + return null; + } + + if ($alias !== null) { + throw new UnexpectedValueException("Unknown alias '{$alias}' given"); + } + + throw new BadFunctionCallException("This fake 'app' should always be called with an alias"); +} + +require __DIR__.'/../../src/OomBootstrapper.php'; + +(new OomBootstrapper())->bootstrap(); + +ini_set('memory_limit', '5M'); + +$i = 0; + +gc_disable(); + +while ($i++ < 12345678) { + $a = new stdClass; + $a->b = $a; +} + +echo "No OOM!\n"; +?> +--SKIPIF-- +<?php +if (PHP_MAJOR_VERSION < 7) { + echo "SKIP - PHP 5 does not run OOM in this test"; +} +?> +--EXPECTF-- +Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line %d +'app' was called! diff --git a/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_present.phpt b/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_present.phpt new file mode 100644 index 00000000..3cfcaeb6 --- /dev/null +++ b/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_present.phpt @@ -0,0 +1,59 @@ +--TEST-- +The OomBootstrapper should not crash when bugsnag is in the DI container +--FILE-- +<?php + +use Bugsnag\BugsnagLaravel\OomBootstrapper; + +class FakeClient +{ + public function getMemoryLimitIncrease() + { + echo "'getMemoryLimitIncrease' was called!\n"; + + return 12345; + } +} + +function app($alias = null) { + echo "'app' was called!\n"; + + if ($alias === 'bugsnag') { + return new FakeClient(); + } + + if ($alias !== null) { + throw new UnexpectedValueException("Unknown alias '{$alias}' given"); + } + + throw new BadFunctionCallException("This fake 'app' should always be called with an alias"); +} + +require __DIR__.'/../../src/OomBootstrapper.php'; + +(new OomBootstrapper())->bootstrap(); + +ini_set('memory_limit', '5M'); + +$i = 0; + +gc_disable(); + +while ($i++ < 12345678) { + $a = new stdClass; + $a->b = $a; +} + +echo "No OOM!\n"; +?> +--SKIPIF-- +<?php +if (PHP_MAJOR_VERSION < 7) { + echo "SKIP - PHP 5 does not run OOM in this test"; +} +?> +--EXPECTF-- +Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line %d +'app' was called! +'getMemoryLimitIncrease' was called! +'getMemoryLimitIncrease' was called!