Skip to content

Commit

Permalink
Improve exception handling: Properly raise IntegrityError exceptions
Browse files Browse the repository at this point in the history
... when receiving `DuplicateKeyException` errors from CrateDB, instead
of the more general `ProgrammingError`.
  • Loading branch information
amotl committed Sep 28, 2023
1 parent 47ad022 commit ea2555c
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Unreleased
``subjectAltName`` attribute will be used.
- SQLAlchemy: Improve DDL compiler to ignore foreign key and uniqueness
constraints
- DBAPI: Properly raise ``IntegrityError`` exceptions instead of
``ProgrammingError``, when CrateDB raises a ``DuplicateKeyException``.

.. _urllib3 v2.0 migration guide: https://urllib3.readthedocs.io/en/latest/v2-migration-guide.html
.. _urllib3 v2.0 roadmap: https://urllib3.readthedocs.io/en/stable/v2-roadmap.html
Expand Down
13 changes: 13 additions & 0 deletions src/crate/client/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
BlobLocationNotFoundException,
DigestNotFoundException,
ProgrammingError,
IntegrityError,
)


Expand Down Expand Up @@ -191,6 +192,18 @@ def _ex_to_message(ex):


def _raise_for_status(response):
"""
Properly raise `IntegrityError` exceptions for CrateDB's `DuplicateKeyException` errors.
"""
try:
return _raise_for_status_real(response)
except ProgrammingError as ex:
if "DuplicateKeyException" in ex.message:
raise IntegrityError(ex.message, error_trace=ex.error_trace) from ex
raise


def _raise_for_status_real(response):
""" make sure that only crate.exceptions are raised that are defined in
the DB-API specification """
message = ''
Expand Down
26 changes: 24 additions & 2 deletions src/crate/client/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@
from setuptools.ssl_support import find_ca_bundle

from .http import Client, CrateJsonEncoder, _get_socket_opts, _remove_certs_for_non_https
from .exceptions import ConnectionError, ProgrammingError

from .exceptions import ConnectionError, ProgrammingError, IntegrityError

REQUEST = 'crate.client.http.Server.request'
CA_CERT_PATH = find_ca_bundle()
Expand Down Expand Up @@ -91,6 +90,17 @@ def bad_bulk_response():
return r


def duplicate_key_exception():
r = fake_response(409, 'Conflict')
r.data = json.dumps({
"error": {
"code": 4091,
"message": "DuplicateKeyException[A document with the same primary key exists already]"
}
}).encode()
return r


def fail_sometimes(*args, **kwargs):
if random.randint(1, 100) % 10 == 0:
raise urllib3.exceptions.MaxRetryError(None, '/_sql', '')
Expand Down Expand Up @@ -302,6 +312,18 @@ def test_uuid_serialization(self, request):
self.assertEqual(data['args'], [str(uid)])
client.close()

@patch(REQUEST, fake_request(duplicate_key_exception()))
def test_duplicate_key_error(self):
"""
Verify that an `IntegrityError` is raised on duplicate key errors,
instead of the more general `ProgrammingError`.
"""
client = Client(servers="localhost:4200")
with self.assertRaises(IntegrityError) as cm:
client.sql('INSERT INTO testdrive (foo) VALUES (42)')
self.assertEqual(cm.exception.message,
"DuplicateKeyException[A document with the same primary key exists already]")


@patch(REQUEST, fail_sometimes)
class ThreadSafeHttpClientTest(TestCase):
Expand Down

0 comments on commit ea2555c

Please sign in to comment.