Fix: ensure db state is reset on exception by adding try-finally blocks to core methods - #1058
Open
ZerfyT wants to merge 1 commit into
Open
Fix: ensure db state is reset on exception by adding try-finally blocks to core methods#1058ZerfyT wants to merge 1 commit into
ZerfyT wants to merge 1 commit into
Conversation
…y blocks to core methods
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DB state isn't reset when catching
mysqli_sql_exception(PHP 8.1+)Since PHP 8.1 now throws
mysqli_sql_exceptionby default for db errors, if a nativemysqlimethod throws an error, the exception bubbles up and completely skips the$this->reset()cleanup call insideMysqliDb.Because of this, if we wrap a query in a
try-catchblock to handle an error in our app, the DB instance gets stuck in a "dirty" state. Internal variables like_bindParamsor_queryOptionsfrom the failed query are never cleared out.Then, when we try to run our next (perfectly valid) query on the same instance, it accidentally reuses those leftover variables. This leads to confusing fatal errors like
ArgumentCountErrorbecause the placeholders and parameters don't match up anymore.Steps to reproduce
Just make sure you're on PHP 8.1+ where mysqli exceptions are on by default.
What actually happens
The second query dies with a fatal error:
Solution
A simple fix is to wrap the main execution blocks (in methods like
get,update,delete,rawQuery,_buildInsert,lock,unlock) inside atry-finally. If an exception hits, we just force a$this->reset()before throwing it back up.Something like this: