#3137883: Deprecate passing a StatementInterface object to Connection::query added a DatabaseExceptionWrapper. The third argument should be the \Throwable and not the code.
throw new DatabaseExceptionWrapper($message, 0, $e->getCode());
Change to throw new DatabaseExceptionWrapper($message, $e->getCode(), $e);
1. Write a test in the database group to assert that bad queries have the correct exception thrown. To start with, let's focus on queries that get caught in DatabaseExceptionWrapper exceptions and not in the other exceptions. This could have a data provider to assert a query on a specific database driver.
2. Make the fix.
3. Review
No
No
No
| Comment | File | Size | Author |
|---|
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
compare
Comments
Comment #2
daffie commented@mradcliffe: Good find! Only I do not think that a novice can easily fix this as we also need add a test for this fix.
Comment #3
mradcliffeI guess writing a test first would be ideal, and that might not be as straightforward. But after that there's no reason a new contributor couldn't provide a review or the fix itself if they wanted to collaborate.
I'll clarify the steps in the issue summary, thank you.
Comment #5
pahles commentedIs there any progress on this issue? I'm running into this error multiple times each day now. Is the proposed solution valid? Can I 'just' (until a real solution exists) patch the file?
Comment #6
mradcliffeThere hasn't been any progress on this issue, but if you're up for it, you can create a patch using a git checkout of drupal core, and then upload it so you can use it on your site AND make progress on the issue at the same time.
Comment #9
adam-vessey commentedSlapped together something of a test; though, not 100% happy with the amount of mocking necessary (without into actually using the DB, that is)... half-tempted to suggest pulling the exception mapping/wrapping out into a separate method that could be more simply targeted for testing this specific issue, but may be excessive.
Comment #10
adam-vessey commentedHmm, had older stuff locally that I was looking at, not realizing that things have been pulled out to separate modules. This... might no longer be an issues, as of
https://www.drupal.org/node/3129492#3185269: Introduce Connection::lastInsertId and deprecate the 'return' query option and Database::RETURN_* constants (https://www.drupal.org/node/3129492 just moved things around, from Drupal core out to modules)?... Indeed seems to be the case, with the wrapping now out in the "ExceptionHandler" (https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/lib/Drupal/C...), being invoked from the "pgsql" module's "Insert" (https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/modules/pgsq...), assuming there's not another "ExceptionHandler" being introduced somewhere which exhibits this issue?
Comment #11
paul121 commented@adam-vessey I believe that is the case, this has been fixed in 9.4. See this comment in #3255437: Avoid error when throwing DatabaseExceptionWrapper for insert queries: https://www.drupal.org/project/drupal/issues/3255437#comment-14560199
Comment #13
daffie commentedThis has been fixed in #3185269: Introduce Connection::lastInsertId and deprecate the 'return' query option and Database::RETURN_* constants.