search

silverstripe / cwp

Common Web Platform (CWP) features module. We strongly recommend using it for all new CWP projects. Future features will be delivered here.
https://www.cwp.govt.nz
BSD 3-Clause "New" or "Revised" License
10 stars 27 forks source link

MNT Handle DatabaseExceptions thrown by PHP 8.1 #311

Closed emteknetnz closed 1 year ago

emteknetnz commented 1 year ago

PHP 8.1 set the MySQLi default error mode to exceptions https://php.watch/versions/8.1/mysqli-error-mode

This resulted in https://github.com/silverstripe/cwp/runs/7351123074?check_suite_focus=true

It would have been nice to just add | DatabaseException to https://github.com/silverstripe/cwp/blob/192a9025ccac935d3bc605fcc56a76002848b4a5/src/PageTypes/DatedUpdateHolderController.php#L178 as talked about here

However the failing FunctionalTest (which makes an HTTP request to the frontend) simply doesn't go through this bit of code.

Incorrect DATETIME value: 'christmas 00:00:00'
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/ORM/Connect/DBConnector.php:64
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/ORM/Connect/MySQLiConnector.php:71
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/ORM/Connect/MySQLiConnector.php:289
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/ORM/Connect/Database.php:185
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/ORM/Connect/Database.php:258
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/ORM/Connect/Database.php:183
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/ORM/Connect/MySQLDatabase.php:402
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/ORM/DB.php:445
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/ORM/Queries/SQLExpression.php:115
/home/runner/work/cwp/cwp/src/PageTypes/DatedUpdateHolder.php:201
/home/runner/work/cwp/cwp/src/PageTypes/DatedUpdateHolderController.php:289
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/ViewableData.php:485
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/ViewableData.php:532
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer_Scope.php:323
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer_DataPresenter.php:309
/tmp/silverstripe-cache-php8.1.7-home-runner-work-cwp-cwp/runner/.cachethemes.starter.templates.CWP.CWP.PageTypes.Layout.EventHolder.ss:46
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer.php:602
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer.php:674
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer.php:668
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer_DataPresenter.php:330
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer_DataPresenter.php:363
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer_DataPresenter.php:165
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer_DataPresenter.php:296
/tmp/silverstripe-cache-php8.1.7-home-runner-work-cwp-cwp/runner/.cachethemes.starter.templates.Page.ss:72
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer.php:602
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/View/SSViewer.php:674
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Controller.php:302
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/RequestHandler.php:202
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Controller.php:212
/home/runner/work/cwp/cwp/vendor/silverstripe/cms/code/Controllers/ContentController.php:251
/home/runner/work/cwp/cwp/vendor/silverstripe/cms/code/Controllers/ModelAsController.php:101
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Director.php:360
/home/runner/work/cwp/cwp/vendor/silverstripe/versioned/src/VersionedHTTPMiddleware.php:41
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/session-manager/src/Middleware/LoginSessionMiddleware.php:29
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/ConfirmationMiddleware.php:254
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/ConfirmationMiddleware.php:254
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Security/PasswordExpirationMiddleware.php:84
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Security/BasicAuthMiddleware.php:68
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Security/AuthenticationMiddleware.php:61
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/ExecMetricMiddleware.php:20
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/CanonicalURLMiddleware.php:190
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPCacheControlMiddleware.php:42
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/ChangeDetectionMiddleware.php:28
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/FlushMiddleware.php:27
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/RequestProcessor.php:66
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/hybridsessions/src/Control/HybridSessionMiddleware.php:18
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/AllowedHostsMiddleware.php:60
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/TrustedProxyMiddleware.php:176
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/cwp/cwp-core/src/Control/InitialisationMiddleware.php:89
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:62
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Middleware/HTTPMiddlewareAware.php:65
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Director.php:369
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Director.php:134
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Director.php:279
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Control/Director.php:133
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Dev/TestSession.php:107
/home/runner/work/cwp/cwp/vendor/silverstripe/framework/src/Dev/FunctionalTest.php:183
/home/runner/work/cwp/cwp/tests/PageTypes/DatedUpdateHolderControllerTest.php:59
/home/runner/work/cwp/cwp/vendor/bin/phpunit:123
GuySartorelli commented 1 year ago

I don't think this actually solves the problem - users are going to get different behaviour depending on what database version is used which is not good. I didn't realise you had already created this PR and was working on the same issue from the recipe-solr-search broken build and created this PR: https://github.com/silverstripe/cwp/pull/312 I think it's better to fix the problem and always return the same error to the user rather than allowing the exception to throw for mysql8.0

emteknetnz commented 1 year ago

Changed solution, this matches behavior to PHP < 8.1