Skip to content

Commit

Permalink
feat: raise any httpx exceptions, e.g. from failed auth
Browse files Browse the repository at this point in the history
This means that 403s, such as those from not being allowed to fetch a specific
version, are passed to client code, and so help debug permission errors better
than the disk i/o error that's being raised right now.

It also means there is consistency - before this change if there's a 403 (or
any other connection error) from the initial HEAD request on the start of a
query, then that was being passed to client now. Now, during the read data
during the query if these get raised they get passed to client code.
  • Loading branch information
michalc committed Mar 15, 2024
1 parent 92267ea commit 9c88aaf
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 4 deletions.
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,13 @@ This means that sqlite-s3-query is not for all use cases of querying SQLite data
This is not necessarily a permanent decision - it is possible that in future sqlite-s3-query will support unversioned buckets.


## Exception hierarchy
## Exceptions

Under the hood [HTTPX](https://www.python-httpx.org/) is used to communicate with S3, but any [exceptions raised by HTTPX](https://www.python-httpx.org/exceptions/) are passed through to client code unchanged. This includes `httpx.HTTPStatusError` when S3 returns a non-200 response. Most commonly this will be when S3 returns a 403 in the case of insufficient permissions on the database object being queried.

All other exceptions raised inherit from `sqlite_s3_query.SQLiteS3QueryError` as described in the following hierarchy.

### Exception hierarchy

- `SQLiteS3QueryError`

Expand Down
26 changes: 24 additions & 2 deletions sqlite_s3_query.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import hmac
import os
import threading
from contextlib import contextmanager
from ctypes import CFUNCTYPE, POINTER, Structure, create_string_buffer, pointer, cast, memmove, memset, sizeof, addressof, cdll, byref, string_at, c_char_p, c_int, c_double, c_int64, c_void_p, c_char
from ctypes.util import find_library
Expand Down Expand Up @@ -69,13 +70,33 @@ def sqlite_s3_query_multi(url, get_credentials=lambda now: (
body_hash = sha256(b'').hexdigest()
scheme, netloc, path, _, _ = urlsplit(url)

# We could use contextvars, but they aren't introduced until Python 3.7
pending_exceptions = {}
pending_exception_lock = threading.Lock()

def set_pending_exception(exception):
thread_id = threading.get_ident()
with pending_exception_lock:
pending_exceptions[thread_id] = exception

def raise_any_pending_exception():
thread_id = threading.get_ident()
with pending_exception_lock:
try:
raise pending_exceptions.pop(thread_id)
except KeyError:
pass

def run(func, *args):
res = func(*args)
raise_any_pending_exception()
if res != 0:
raise SQLiteError(libsqlite3.sqlite3_errstr(res).decode())

def run_with_db(db, func, *args):
if func(*args) != 0:
res = func(*args)
raise_any_pending_exception()
if res != 0:
raise SQLiteError(libsqlite3.sqlite3_errmsg(db).decode())

@contextmanager
Expand Down Expand Up @@ -183,7 +204,8 @@ def x_read(p_file, p_out, i_amt, i_ofst):
offset += len(chunk)
if offset > i_amt:
break
except Exception:
except Exception as exception:
set_pending_exception(exception)
return SQLITE_IOERR

if offset != i_amt:
Expand Down
38 changes: 37 additions & 1 deletion test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import uuid

import httpx
from httpx import HTTPStatusError

from sqlite_s3_query import (
VersioningNotEnabledError,
Expand Down Expand Up @@ -411,7 +412,42 @@ def test_empty_object(self):
'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY',
None,
), get_libsqlite3=get_libsqlite3) as query:
with self.assertRaisesRegex(SQLiteError, 'disk I/O error'):
with self.assertRaisesRegex(HTTPStatusError, r"\b416\b"):
query('SELECT 1').__enter__()

def test_incorrect_permission_on_context_enter(self):
with get_db([("CREATE TABLE my_table (my_col_a text, my_col_b text);",())]) as db:
put_object_with_versioning('my-bucket', 'my.db', db)

with self.assertRaisesRegex(HTTPStatusError, r"\b403\b"):
sqlite_s3_query('http://localhost:9000/my-bucket/my.db', get_credentials=lambda now: (
'us-east-1',
'AKIAIOSFODNN7EXAMPLE',
'not-the-right-key',
None,
), get_libsqlite3=get_libsqlite3).__enter__()

def test_incorrect_permission_on_run_query(self):
with get_db([("CREATE TABLE my_table (my_col_a text, my_col_b text);",())]) as db:
put_object_with_versioning('my-bucket', 'my.db', db)

creds = (
(
'us-east-1',
'AKIAIOSFODNN7EXAMPLE',
'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY',
None,
), (
'us-east-1',
'AKIAIOSFODNN7EXAMPLE',
'not-the-right-key',
None,
)
)
creds_it = iter(creds)

with sqlite_s3_query('http://localhost:9000/my-bucket/my.db', get_credentials=lambda now: next(creds_it), get_libsqlite3=get_libsqlite3) as query:
with self.assertRaisesRegex(HTTPStatusError, r"\b403\b"):
query('SELECT 1').__enter__()

def test_bad_db_header(self):
Expand Down

0 comments on commit 9c88aaf

Please sign in to comment.