Problem/Motivation

In 9.2 we have created the ExceptionHandler class. See: https://www.drupal.org/node/3187222. This driver should do the same.

Steps to reproduce

Proposed resolution

Add the class ExceptionHandler and start using it.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 3244841-2.patch7.57 KBdaffie

Issue fork sqlsrv-3244841

Command icon Show commands

Start 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:

About issue forks

Comments

daffie created an issue. See original summary.

daffie’s picture

daffie commented
Status: Active » Needs review
StatusFileSize
new 3244841-2.patch7.57 KB

The fix. In this patch is also a small fix for the fact that in the class Insert the following code is deprecated: return $stmt->fetchColumn(0);. I have changed it to: return $stmt->fetchField();. The parameter value does not need to be set, because it is the default value.

beakerboy’s picture

beakerboy
Primary language English
Location Kentucky
commented

Does the lack of ExceptionHandler cause any test failures?

daffie’s picture

daffie commented

Does the lack of ExceptionHandler cause any test failures?

Yes, they do. I can find out for you if you need it.

beakerboy’s picture

beakerboy
Primary language English
Location Kentucky
commented

Yes, please. I'd like to run just a failing test with and without the patch first to make sure it gets resolved before running the entire suite. For some reason the patch is not applying. Could you either reroll it against the existing 4.3.x branch or pass on a merge request on the issue fork? It looks like Truncate and Update are the problems.

Edit: never mind. It looks like this patch contains some of the changes from the first patch you submitted. It doesn’t apply because those changes have already been made. I think the git apply —reject option will take care of it.

Beakerboy opened merge request !26

  • Beakerboy committed 936673c on 4.3.x authored by daffie 
    Issue #3244841 by daffie: Add the class ExceptionHandler
daffie’s picture

daffie commented
Status: Needs review » Fixed

Has been committed.

beakerboy’s picture

beakerboy
Primary language English
Location Kentucky
commented

Has been Cherry-picked into 4.2.x as well. The new file has been added, but nothing else yet.

beakerboy’s picture

beakerboy
Primary language English
Location Kentucky
commented

I think the provided patch has a bug:
// Match all SQLSTATE 23xxx errors.
+ if (substr($e->getCode(), -6, -3) == '23') {
+ throw new IntegrityConstraintViolationException($message, $e->getCode(), $e);
+ }
+ else {
+ throw new DatabaseExceptionWrapper($message, 0, $e);
+ }

This seems like it was copied/pasted from psql driver. The code in the ExceptionWrapper uses '42' instead of '23'. which is correct? why not just use:

      try {
        // Run the query.
        $stmt->execute(NULL, $options);
      }
      catch (\Exception $e) {
        $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, [], $this->queryOptions);
      }

I'm going to have to look closer at these patches before merging...

beakerboy’s picture

beakerboy
Primary language English
Location Kentucky
commented
Status: Fixed » Needs work

  • Beakerboy committed 1a4df03 on 4.2.x 
    Issue #3244841 by Beakerboy, daffie: Add the class ExceptionHandler

  • Beakerboy committed 5531c01 on 4.2.x 
    Issue #3244841 by Beakerboy, daffie: Add the class ExceptionHandler
beakerboy’s picture

beakerboy
Primary language English
Location Kentucky
commented

Now to fix 4.3.

beakerboy’s picture

beakerboy
Primary language English
Location Kentucky
commented
Status: Needs work » Fixed

  • Beakerboy committed ee653bf on 4.3.x 
    Issue #3244841 by Beakerboy, daffie: Add the class ExceptionHandler

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.