fix(error): return HTTP 500 for uncaught frontend errors - #16982
fix(error): return HTTP 500 for uncaught frontend errors#16982Ibochkarev wants to merge 3 commits into
Conversation
Register exception and shutdown handlers so TypeErrors, fatal errors,
and E_USER_ERROR during web requests produce HTTP 500 via sendError('fatal')
instead of a blank HTTP 200 response.
Fixes modxcms#16932
Make the handler constructor public for direct unit testing and run the fatal exception test in a separate process so headers_sent() stays false.
|
Thanks for this PR @Ibochkarev. I haven't tested so this is a question where I don't know the answer: if I switch the logger implementation to Monolog and register its exception handler, which one wins here? |
Good question. #16931 only adds This PR registers
|
What changed and why
When a snippet, plugin, TV, or other element throws during web rendering, MODX often returned a blank page with HTTP 200. Proxies cached that response. Uptime checks saw 200 and missed the failure.
This PR adds
modUncaughtErrorHandler, registered from_initErrorHandler():set_exception_handler()for uncaught\Throwable(TypeError,ParseError, etc.)register_shutdown_function()witherror_get_last()forE_ERROR,E_PARSE,E_CORE_ERROR,E_COMPILE_ERRORE_USER_ERRORinmodErrorHandlerroutes toLOG_LEVEL_FATALon web requests whensend_http_500_on_erroris enabledEach path calls the existing
sendError('fatal')flow.fatal.include.phpnow sends no-cache headers so reverse proxies do not store 500 responses.Opt out with system setting
send_http_500_on_error(defaulttrue).How to test
TypeError(pass a string to a typedintparameter).[[snippetName]]on a published resource and open the page.Cache-Control: no-store.send_http_500_on_errortofalseand confirm the handler no longer forces 500.Unit tests:
CI (phpcs, PHPUnit on MySQL 8.1–8.5) is green on this branch.
Related issue(s)/PR(s)
Resolves #16932
Compatibility notes
sendError()behavior in CLI is unchanged.modX.\Throwablehierarchy). No new Composer dependencies.Breaking change assessment
Behavior change:
E_USER_ERRORon web requests now terminates with HTTP 500 whensend_http_500_on_erroris enabled (default). Code that logged a user error and kept running should useE_USER_WARNINGor disable the setting.No public API signature changes. Patch-safe for sites that do not rely on
E_USER_ERRORcontinuing after the handler runs.Test coverage
_build/test/Tests/Model/Error/modUncaughtErrorHandlerTest.php: fatal type detection, error formatting, exception logging, shutdown behavior for non-fatal errors, registration (web only)._build/test/Tests/Model/Error/modErrorHandlerTest.php:E_USER_ERROR→LOG_LEVEL_FATALwhen enabled (web only; skipped under CLI PHPUnit).Contributors
Thanks to @pbowyer for #16932 and the documented workaround that shaped this approach.
AI tool use
Claude (Cursor) helped draft the implementation, tests, and PR text from the issue analysis and codebase review.