Skip to content

Commit a02f399

Browse files
authored
chore: improve plaintext error handling (#462)
1 parent 7ba2643 commit a02f399

5 files changed

Lines changed: 56 additions & 4 deletions

File tree

src/firebolt/utils/util.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
from firebolt.utils.exception import (
2121
ConfigurationError,
22+
FireboltError,
2223
FireboltStructuredError,
2324
)
2425

@@ -171,6 +172,10 @@ def raise_error_from_response(resp: Response) -> None:
171172
resp (Response): HTTP response
172173
"""
173174
to_raise = None
175+
# If error is Text - raise as is
176+
if "text/plain" in resp.headers.get("Content-Type", ""):
177+
raise FireboltError(resp.text)
178+
# If error is Json - parse it and raise
174179
try:
175180
decoded = resp.json()
176181
if "errors" in decoded and len(decoded["errors"]) > 0:
@@ -186,6 +191,7 @@ def raise_error_from_response(resp: Response) -> None:
186191
raise to_raise
187192

188193
# Raise status error if no error info was found in the body
194+
# This error does not contain the response body
189195
resp.raise_for_status()
190196

191197

tests/integration/dbapi/async/V2/test_queries_async.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from firebolt.client.auth.base import Auth
1414
from firebolt.common._types import ColType
1515
from firebolt.common.row_set.types import Column
16-
from firebolt.utils.exception import FireboltStructuredError
16+
from firebolt.utils.exception import FireboltError, FireboltStructuredError
1717
from tests.integration.dbapi.conftest import LONG_SELECT_DEFAULT_V2
1818
from tests.integration.dbapi.utils import assert_deep_eq
1919

@@ -435,7 +435,7 @@ async def test_multi_statement_query(connection: Connection) -> None:
435435
async def test_set_invalid_parameter(connection: Connection):
436436
async with connection.cursor() as c:
437437
assert len(c._set_parameters) == 0
438-
with raises((OperationalError, FireboltStructuredError)) as e:
438+
with raises((OperationalError, FireboltError)) as e:
439439
await c.execute("SET some_invalid_parameter = 1")
440440

441441
assert "Unknown setting" in str(e.value) or "query param not allowed" in str(

tests/integration/dbapi/sync/V2/test_queries.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from firebolt.common._types import ColType
1414
from firebolt.common.row_set.types import Column
1515
from firebolt.db import Binary, Connection, Cursor, OperationalError, connect
16-
from firebolt.utils.exception import FireboltStructuredError
16+
from firebolt.utils.exception import FireboltError, FireboltStructuredError
1717
from tests.integration.dbapi.conftest import LONG_SELECT_DEFAULT_V2
1818
from tests.integration.dbapi.utils import assert_deep_eq
1919

@@ -435,7 +435,7 @@ def test_multi_statement_query(connection: Connection) -> None:
435435
def test_set_invalid_parameter(connection: Connection):
436436
with connection.cursor() as c:
437437
assert len(c._set_parameters) == 0
438-
with raises((OperationalError, FireboltStructuredError)) as e:
438+
with raises((OperationalError, FireboltError)) as e:
439439
c.execute("set some_invalid_parameter = 1")
440440
assert "Unknown setting" in str(e.value) or "query param not allowed" in str(
441441
e.value

tests/unit/async_db/test_cursor.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,3 +1915,26 @@ async def test_transaction_params_included_in_query_requests(
19151915
)
19161916

19171917
await cursor.execute("SELECT 2")
1918+
1919+
1920+
async def test_cursor_plaintext_error(
1921+
httpx_mock: HTTPXMock,
1922+
cursor: Cursor,
1923+
query_url: str,
1924+
):
1925+
"""Test handling of plaintext error responses from the server."""
1926+
httpx_mock.add_callback(
1927+
lambda *args, **kwargs: Response(
1928+
status_code=codes.NOT_FOUND,
1929+
text="Plaintext error message",
1930+
headers={"Content-Type": "text/plain"},
1931+
),
1932+
url=query_url,
1933+
)
1934+
with raises(FireboltError) as excinfo:
1935+
await cursor.execute("select * from t")
1936+
1937+
assert cursor._state == CursorState.ERROR
1938+
assert "Plaintext error message" in str(
1939+
excinfo.value
1940+
), "Invalid error message for plaintext error response"

tests/unit/db/test_cursor.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,3 +1803,26 @@ def test_transaction_params_included_in_query_requests(
18031803

18041804
# This will only succeed if transaction parameters are properly passed
18051805
cursor.execute("SELECT 2")
1806+
1807+
1808+
def test_cursor_plaintext_error(
1809+
httpx_mock: HTTPXMock,
1810+
cursor: Cursor,
1811+
query_url: str,
1812+
):
1813+
"""Test handling of plaintext error responses from the server."""
1814+
httpx_mock.add_callback(
1815+
lambda *args, **kwargs: Response(
1816+
status_code=codes.NOT_FOUND,
1817+
text="Plaintext error message",
1818+
headers={"Content-Type": "text/plain"},
1819+
),
1820+
url=query_url,
1821+
)
1822+
with raises(FireboltError) as excinfo:
1823+
cursor.execute("select * from t")
1824+
1825+
assert cursor._state == CursorState.ERROR
1826+
assert "Plaintext error message" in str(
1827+
excinfo.value
1828+
), "Invalid error message for plaintext error response"

0 commit comments

Comments
 (0)