From 9c88aafb34a08de2ec2040bca06e12b18f60cbc3 Mon Sep 17 00:00:00 2001 From: Michal Charemza Date: Fri, 15 Mar 2024 19:09:30 +0000 Subject: [PATCH] feat: raise any httpx exceptions, e.g. from failed auth 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. --- README.md | 8 +++++++- sqlite_s3_query.py | 26 ++++++++++++++++++++++++-- test.py | 38 +++++++++++++++++++++++++++++++++++++- 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 3a357bb..9a20f06 100644 --- a/README.md +++ b/README.md @@ -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` diff --git a/sqlite_s3_query.py b/sqlite_s3_query.py index 2c5e64e..8dd1f23 100644 --- a/sqlite_s3_query.py +++ b/sqlite_s3_query.py @@ -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 @@ -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 @@ -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: diff --git a/test.py b/test.py index 167a263..d8d079e 100644 --- a/test.py +++ b/test.py @@ -15,6 +15,7 @@ import uuid import httpx +from httpx import HTTPStatusError from sqlite_s3_query import ( VersioningNotEnabledError, @@ -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):