mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-13 14:16:54 +00:00
fix(python): surface typed remote resource errors (#4789)
## Summary - Parse structured `error_code` and `resource` fields from remote HTTP error bodies and carry them through Rust/PyO3. - Add `SecretNotFoundError`, `FunctionNotFoundError`, `NamespaceNotFoundError`, and `AlreadyExistsError` as `HttpError` subclasses. - Expose `error_code`, `resource_kind`, `resource_name`, and `namespace_path` on HTTP exceptions while preserving `status_code`, `request_id`, and existing `except HttpError` handlers. - Do not retry structured domain errors that would otherwise become `RetryError` (notably `already_exists` on 409). Older servers that do not provide the structured fields continue to raise a plain `HttpError`. ## Compatibility This consumes the stable resource error contract added in lancedb/sophon#9516 (merge commit `35858561b3`). The existing `TableNotFoundError` behavior is unchanged. Part of ENT-3373.
This commit is contained in:
@@ -748,10 +748,45 @@ so the fetch APIs cannot read their bytes yet.
|
||||
|
||||
## Exceptions
|
||||
|
||||
Remote HTTP responses with structured resource details raise a subclass of
|
||||
`HttpError`. Older servers that omit those details raise plain `HttpError`, so
|
||||
`except HttpError` remains a compatible catch-all:
|
||||
|
||||
```python
|
||||
from lancedb.remote.errors import (
|
||||
AlreadyExistsError,
|
||||
FunctionNotFoundError,
|
||||
HttpError,
|
||||
NamespaceNotFoundError,
|
||||
SecretNotFoundError,
|
||||
)
|
||||
|
||||
try:
|
||||
db.describe_namespace(["analytics"])
|
||||
except (SecretNotFoundError, FunctionNotFoundError, NamespaceNotFoundError) as error:
|
||||
print(error.error_code, error.resource_kind, error.resource_name)
|
||||
except AlreadyExistsError as error:
|
||||
print("resource already exists:", error.resource_name)
|
||||
except HttpError as error:
|
||||
print("request failed:", error.request_id, error.status_code)
|
||||
```
|
||||
|
||||
Secret and function lifecycle APIs currently use Flight SQL and may still raise
|
||||
`RuntimeError`. The typed subclasses cover HTTP failures, including dependency
|
||||
errors during column refresh.
|
||||
|
||||
::: lancedb.remote.errors.HttpError
|
||||
|
||||
::: lancedb.remote.errors.TableNotFoundError
|
||||
|
||||
::: lancedb.remote.errors.SecretNotFoundError
|
||||
|
||||
::: lancedb.remote.errors.FunctionNotFoundError
|
||||
|
||||
::: lancedb.remote.errors.NamespaceNotFoundError
|
||||
|
||||
::: lancedb.remote.errors.AlreadyExistsError
|
||||
|
||||
::: lancedb.exceptions.MissingValueError
|
||||
|
||||
::: lancedb.exceptions.MissingColumnError
|
||||
|
||||
@@ -46,9 +46,93 @@ class HttpError(LanceDBClientError):
|
||||
status_code: int
|
||||
The HTTP status code of the response. May be None if the request
|
||||
failed before the response was received.
|
||||
error_code: str
|
||||
A machine-readable error code reported by the server, if available.
|
||||
resource_kind: str
|
||||
The kind of resource reported by the server, if available.
|
||||
resource_name: str
|
||||
The name of the resource reported by the server, if available.
|
||||
namespace_path: tuple[str, ...]
|
||||
The parent namespace path of the resource, if available.
|
||||
"""
|
||||
|
||||
pass
|
||||
def __new__(
|
||||
cls,
|
||||
message: str,
|
||||
request_id: str,
|
||||
status_code: Optional[int] = None,
|
||||
error_code: Optional[str] = None,
|
||||
resource_kind: Optional[str] = None,
|
||||
resource_name: Optional[str] = None,
|
||||
namespace_path: Optional[tuple[str, ...]] = None,
|
||||
):
|
||||
if cls is HttpError:
|
||||
mapped_cls = _http_error_class(error_code, resource_kind)
|
||||
if mapped_cls is not None:
|
||||
cls = mapped_cls
|
||||
return RuntimeError.__new__(cls)
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str,
|
||||
request_id: str,
|
||||
status_code: Optional[int] = None,
|
||||
error_code: Optional[str] = None,
|
||||
resource_kind: Optional[str] = None,
|
||||
resource_name: Optional[str] = None,
|
||||
namespace_path: Optional[tuple[str, ...]] = None,
|
||||
):
|
||||
super().__init__(message, request_id, status_code)
|
||||
self.error_code = error_code
|
||||
self.resource_kind = resource_kind
|
||||
self.resource_name = resource_name
|
||||
self.namespace_path = (
|
||||
tuple(namespace_path) if namespace_path is not None else None
|
||||
)
|
||||
|
||||
def __reduce__(self) -> tuple[type, tuple]:
|
||||
return (
|
||||
self.__class__,
|
||||
(
|
||||
str(self),
|
||||
self.request_id,
|
||||
self.status_code,
|
||||
self.error_code,
|
||||
self.resource_kind,
|
||||
self.resource_name,
|
||||
self.namespace_path,
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def _http_error_class(
|
||||
error_code: Optional[str], resource_kind: Optional[str]
|
||||
) -> Optional[type[HttpError]]:
|
||||
if error_code == "already_exists":
|
||||
return AlreadyExistsError
|
||||
if error_code != "not_found" or resource_kind is None:
|
||||
return None
|
||||
return {
|
||||
"secret": SecretNotFoundError,
|
||||
"function": FunctionNotFoundError,
|
||||
"namespace": NamespaceNotFoundError,
|
||||
}.get(resource_kind)
|
||||
|
||||
|
||||
class SecretNotFoundError(HttpError):
|
||||
"""A remote secret was not found."""
|
||||
|
||||
|
||||
class FunctionNotFoundError(HttpError):
|
||||
"""A remote function was not found."""
|
||||
|
||||
|
||||
class NamespaceNotFoundError(HttpError):
|
||||
"""A remote namespace was not found."""
|
||||
|
||||
|
||||
class AlreadyExistsError(HttpError):
|
||||
"""A remote resource already exists."""
|
||||
|
||||
|
||||
class TableNotFoundError(HttpError, ValueError, _NamespaceTableNotFoundError):
|
||||
|
||||
@@ -9,9 +9,13 @@ from lance_namespace.errors import TableNotFoundError as NamespaceTableNotFoundE
|
||||
|
||||
from lancedb.db import DBConnection
|
||||
from lancedb.remote.errors import (
|
||||
AlreadyExistsError,
|
||||
FunctionNotFoundError,
|
||||
HttpError,
|
||||
LanceDBClientError,
|
||||
NamespaceNotFoundError,
|
||||
RetryError,
|
||||
SecretNotFoundError,
|
||||
TableNotFoundError,
|
||||
)
|
||||
from lancedb.table import Table
|
||||
@@ -59,6 +63,97 @@ def test_pickle_http_error():
|
||||
assert restored.status_code == 404
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"error_code,resource_kind,status_code,expected_type",
|
||||
[
|
||||
("not_found", "secret", 404, SecretNotFoundError),
|
||||
("not_found", "function", 404, FunctionNotFoundError),
|
||||
("not_found", "namespace", 404, NamespaceNotFoundError),
|
||||
("already_exists", "secret", 409, AlreadyExistsError),
|
||||
],
|
||||
)
|
||||
def test_http_error_structured_resource_error(
|
||||
error_code, resource_kind, status_code, expected_type
|
||||
):
|
||||
err = HttpError(
|
||||
"resource request failed",
|
||||
"req-structured",
|
||||
status_code,
|
||||
error_code,
|
||||
resource_kind,
|
||||
"resource-name",
|
||||
["parent", "child"],
|
||||
)
|
||||
|
||||
assert type(err) is expected_type
|
||||
assert isinstance(err, HttpError)
|
||||
assert err.error_code == error_code
|
||||
assert err.resource_kind == resource_kind
|
||||
assert err.resource_name == "resource-name"
|
||||
assert err.namespace_path == ("parent", "child")
|
||||
assert err.request_id == "req-structured"
|
||||
assert err.status_code == status_code
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"error_code,resource_kind",
|
||||
[
|
||||
(None, None),
|
||||
("not_found", None),
|
||||
("not_found", "unknown"),
|
||||
("invalid", "secret"),
|
||||
],
|
||||
)
|
||||
def test_http_error_without_known_structured_fields_stays_generic(
|
||||
error_code, resource_kind
|
||||
):
|
||||
err = HttpError(
|
||||
"not found",
|
||||
"req-generic",
|
||||
404,
|
||||
error_code,
|
||||
resource_kind,
|
||||
"resource-name",
|
||||
[],
|
||||
)
|
||||
|
||||
assert type(err) is HttpError
|
||||
assert err.error_code == error_code
|
||||
assert err.resource_kind == resource_kind
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"exception_type,error_code,resource_kind",
|
||||
[
|
||||
(SecretNotFoundError, "not_found", "secret"),
|
||||
(FunctionNotFoundError, "not_found", "function"),
|
||||
(NamespaceNotFoundError, "not_found", "namespace"),
|
||||
(AlreadyExistsError, "already_exists", "namespace"),
|
||||
],
|
||||
)
|
||||
def test_pickle_structured_http_error(exception_type, error_code, resource_kind):
|
||||
err = HttpError(
|
||||
"resource request failed",
|
||||
"req-pickle",
|
||||
409 if error_code == "already_exists" else 404,
|
||||
error_code,
|
||||
resource_kind,
|
||||
"resource-name",
|
||||
["parent"],
|
||||
)
|
||||
assert type(err) is exception_type
|
||||
|
||||
restored = pickle.loads(pickle.dumps(err))
|
||||
|
||||
assert type(restored) is exception_type
|
||||
assert restored.error_code == error_code
|
||||
assert restored.resource_kind == resource_kind
|
||||
assert restored.resource_name == "resource-name"
|
||||
assert restored.namespace_path == ("parent",)
|
||||
assert restored.request_id == "req-pickle"
|
||||
assert restored.status_code == err.status_code
|
||||
|
||||
|
||||
def test_pickle_table_not_found_error():
|
||||
err = TableNotFoundError("Table 'missing' was not found", "req-missing", 404)
|
||||
restored = pickle.loads(pickle.dumps(err))
|
||||
|
||||
@@ -42,7 +42,12 @@ from lancedb.query import (
|
||||
MatchQuery,
|
||||
)
|
||||
from lancedb.remote import ClientConfig
|
||||
from lancedb.remote.errors import HttpError, RetryError
|
||||
from lancedb.remote.errors import (
|
||||
AlreadyExistsError,
|
||||
HttpError,
|
||||
NamespaceNotFoundError,
|
||||
RetryError,
|
||||
)
|
||||
import pytest
|
||||
import pyarrow as pa
|
||||
from utils import start_server_thread
|
||||
@@ -258,6 +263,111 @@ async def test_async_remote_exists(method, identifier, status, existence_handler
|
||||
assert exc_info.value.status_code == status
|
||||
|
||||
|
||||
def test_remote_describe_namespace_typed_not_found():
|
||||
request_ids = []
|
||||
|
||||
def handler(request):
|
||||
assert request.command == "POST"
|
||||
assert request.path == "/v1/namespace/parent$child/describe"
|
||||
request_ids.append(request.headers["x-request-id"])
|
||||
request.send_response(404)
|
||||
request.send_header("Content-Type", "application/json")
|
||||
request.end_headers()
|
||||
request.wfile.write(
|
||||
json.dumps(
|
||||
{
|
||||
"code": 5,
|
||||
"error": "Namespace not found",
|
||||
"error_code": "not_found",
|
||||
"resource": {
|
||||
"kind": "namespace",
|
||||
"name": "child",
|
||||
"namespace_path": ["parent"],
|
||||
},
|
||||
}
|
||||
).encode()
|
||||
)
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
with pytest.raises(NamespaceNotFoundError) as exc_info:
|
||||
db.describe_namespace(["parent", "child"])
|
||||
|
||||
error = exc_info.value
|
||||
assert isinstance(error, HttpError)
|
||||
assert error.error_code == "not_found"
|
||||
assert error.resource_kind == "namespace"
|
||||
assert error.resource_name == "child"
|
||||
assert error.namespace_path == ("parent",)
|
||||
assert error.status_code == 404
|
||||
assert error.request_id == request_ids[-1]
|
||||
|
||||
|
||||
def test_remote_create_namespace_typed_already_exists():
|
||||
requests = []
|
||||
|
||||
def handler(request):
|
||||
requests.append(request.path)
|
||||
assert request.command == "POST"
|
||||
assert request.path == "/v1/namespace/parent$child/create"
|
||||
request.send_response(409)
|
||||
request.send_header("Content-Type", "application/json")
|
||||
request.end_headers()
|
||||
request.wfile.write(
|
||||
json.dumps(
|
||||
{
|
||||
"code": 6,
|
||||
"error": "Namespace already exists",
|
||||
"error_code": "already_exists",
|
||||
"resource": {
|
||||
"kind": "namespace",
|
||||
"name": "child",
|
||||
"namespace_path": ["parent"],
|
||||
},
|
||||
}
|
||||
).encode()
|
||||
)
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
with pytest.raises(AlreadyExistsError) as exc_info:
|
||||
db.create_namespace(["parent", "child"])
|
||||
|
||||
error = exc_info.value
|
||||
assert error.error_code == "already_exists"
|
||||
assert error.resource_kind == "namespace"
|
||||
assert error.resource_name == "child"
|
||||
assert error.namespace_path == ("parent",)
|
||||
assert error.status_code == 409
|
||||
assert len(requests) == 1
|
||||
|
||||
|
||||
def test_remote_legacy_namespace_error_stays_generic_http_error():
|
||||
def handler(request):
|
||||
assert request.path == "/v1/namespace/parent$child/describe"
|
||||
request.send_response(404)
|
||||
request.send_header("Content-Type", "application/json")
|
||||
request.end_headers()
|
||||
request.wfile.write(
|
||||
json.dumps(
|
||||
{
|
||||
"code": 5,
|
||||
"error": "Namespace not found",
|
||||
}
|
||||
).encode()
|
||||
)
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
with pytest.raises(HttpError) as exc_info:
|
||||
db.describe_namespace(["parent", "child"])
|
||||
|
||||
error = exc_info.value
|
||||
assert type(error) is HttpError
|
||||
assert error.error_code is None
|
||||
assert error.resource_kind is None
|
||||
assert error.resource_name is None
|
||||
assert error.namespace_path is None
|
||||
assert error.status_code == 404
|
||||
|
||||
|
||||
@pytest.mark.parametrize("table_id", [[], ["invalid/name"]])
|
||||
def test_remote_table_exists_rejects_invalid_identifier(table_id):
|
||||
requests = []
|
||||
|
||||
+36
-10
@@ -2,13 +2,13 @@
|
||||
// SPDX-FileCopyrightText: Copyright The LanceDB Authors
|
||||
|
||||
use pyo3::{
|
||||
PyErr, PyResult, Python,
|
||||
Bound, PyAny, PyErr, PyResult, Python,
|
||||
exceptions::{PyIOError, PyNotImplementedError, PyOSError, PyRuntimeError, PyValueError},
|
||||
intern,
|
||||
types::{PyAnyMethods, PyNone},
|
||||
};
|
||||
|
||||
use lancedb::error::Error as LanceError;
|
||||
use lancedb::error::{Error as LanceError, HttpErrorDetails};
|
||||
|
||||
pub trait PythonErrorExt<T> {
|
||||
/// Convert to a python error based on the Lance error type
|
||||
@@ -65,14 +65,13 @@ impl<T> PythonErrorExt<T> for std::result::Result<T, LanceError> {
|
||||
status_code,
|
||||
} => Python::attach(|py| {
|
||||
let message = err.to_string();
|
||||
let http_err_cls = py
|
||||
.import(intern!(py, "lancedb.remote.errors"))?
|
||||
.getattr(intern!(py, "HttpError"))?;
|
||||
let err = http_err_cls.call1((
|
||||
let err = new_py_http_error(
|
||||
py,
|
||||
message,
|
||||
request_id,
|
||||
status_code.map(|s| s.as_u16()),
|
||||
))?;
|
||||
err.http_error_details(),
|
||||
)?;
|
||||
|
||||
if let Some(cause) = source.source() {
|
||||
// The HTTP error already includes the first cause. But
|
||||
@@ -163,13 +162,15 @@ impl<T> PythonErrorExt<T> for std::result::Result<T, LanceError> {
|
||||
|
||||
fn http_from_rust_error(
|
||||
py: Python<'_>,
|
||||
err: &dyn std::error::Error,
|
||||
err: &(dyn std::error::Error + 'static),
|
||||
request_id: &str,
|
||||
status_code: Option<u16>,
|
||||
) -> PyResult<PyErr> {
|
||||
let message = err.to_string();
|
||||
let http_err_cls = py.import("lancedb.remote.errors")?.getattr("HttpError")?;
|
||||
let py_err = http_err_cls.call1((message, request_id, status_code))?;
|
||||
let details = err
|
||||
.downcast_ref::<LanceError>()
|
||||
.and_then(LanceError::http_error_details);
|
||||
let py_err = new_py_http_error(py, message, request_id, status_code, details)?;
|
||||
|
||||
// Reset the traceback since it doesn't provide additional information.
|
||||
let py_err = py_err.call_method1(intern!(py, "with_traceback"), (PyNone::get(py),))?;
|
||||
@@ -181,3 +182,28 @@ fn http_from_rust_error(
|
||||
|
||||
Ok(PyErr::from_value(py_err))
|
||||
}
|
||||
|
||||
fn new_py_http_error<'py>(
|
||||
py: Python<'py>,
|
||||
message: String,
|
||||
request_id: &str,
|
||||
status_code: Option<u16>,
|
||||
details: Option<&HttpErrorDetails>,
|
||||
) -> PyResult<Bound<'py, PyAny>> {
|
||||
let error_code = details.map(|details| details.error_code.as_str());
|
||||
let resource_kind = details.map(|details| details.resource_kind.as_str());
|
||||
let resource_name = details.map(|details| details.resource_name.as_str());
|
||||
let namespace_path = details.map(|details| details.namespace_path.clone());
|
||||
|
||||
py.import("lancedb.remote.errors")?
|
||||
.getattr("HttpError")?
|
||||
.call1((
|
||||
message,
|
||||
request_id,
|
||||
status_code,
|
||||
error_code,
|
||||
resource_kind,
|
||||
resource_name,
|
||||
namespace_path,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -6,10 +6,78 @@ use std::sync::{Arc, PoisonError};
|
||||
|
||||
use arrow_schema::ArrowError;
|
||||
use datafusion_common::DataFusionError;
|
||||
#[cfg(feature = "remote")]
|
||||
use serde::Deserialize;
|
||||
use snafu::Snafu;
|
||||
|
||||
pub(crate) type BoxError = Box<dyn std::error::Error + Send + Sync>;
|
||||
|
||||
/// Stable resource details carried by a structured remote HTTP error.
|
||||
#[cfg(feature = "remote")]
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
#[non_exhaustive]
|
||||
pub struct HttpErrorDetails {
|
||||
/// Machine-readable error code from the server.
|
||||
pub error_code: String,
|
||||
/// The kind of resource named by the error.
|
||||
pub resource_kind: String,
|
||||
/// The leaf name of the resource.
|
||||
pub resource_name: String,
|
||||
/// The parent namespace path of the resource.
|
||||
pub namespace_path: Vec<String>,
|
||||
}
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct HttpErrorSource {
|
||||
message: String,
|
||||
details: Option<HttpErrorDetails>,
|
||||
}
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
impl HttpErrorSource {
|
||||
pub(crate) fn new(message: String, response_body: Option<&str>) -> Self {
|
||||
let details = response_body.and_then(parse_http_error_details);
|
||||
Self { message, details }
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
impl Display for HttpErrorSource {
|
||||
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
|
||||
f.write_str(&self.message)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
impl std::error::Error for HttpErrorSource {}
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
#[derive(Deserialize)]
|
||||
struct HttpResponseBody {
|
||||
error_code: String,
|
||||
resource: HttpResponseResource,
|
||||
}
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
#[derive(Deserialize)]
|
||||
struct HttpResponseResource {
|
||||
kind: String,
|
||||
name: String,
|
||||
namespace_path: Vec<String>,
|
||||
}
|
||||
|
||||
#[cfg(feature = "remote")]
|
||||
fn parse_http_error_details(response_body: &str) -> Option<HttpErrorDetails> {
|
||||
let body: HttpResponseBody = serde_json::from_str(response_body).ok()?;
|
||||
Some(HttpErrorDetails {
|
||||
error_code: body.error_code,
|
||||
resource_kind: body.resource.kind,
|
||||
resource_name: body.resource.name,
|
||||
namespace_path: body.resource.namespace_path,
|
||||
})
|
||||
}
|
||||
|
||||
// reqwest's Display omits the timeout cause. Add guidance at display time so
|
||||
// callers still receive the original typed source and its full error chain.
|
||||
#[cfg(feature = "remote")]
|
||||
@@ -244,6 +312,23 @@ fn has_unsupported_local_filesystem_source(error: &(dyn std::error::Error + 'sta
|
||||
}
|
||||
|
||||
impl Error {
|
||||
/// Structured resource details reported by a remote HTTP error.
|
||||
///
|
||||
/// Older servers do not include these fields, in which case this returns
|
||||
/// `None` and callers should treat the error as a generic HTTP failure.
|
||||
#[cfg(feature = "remote")]
|
||||
pub fn http_error_details(&self) -> Option<&HttpErrorDetails> {
|
||||
match self {
|
||||
Self::Http { source, .. } => source
|
||||
.downcast_ref::<HttpErrorSource>()
|
||||
.and_then(|source| source.details.as_ref()),
|
||||
Self::Retry { source, .. } => source
|
||||
.downcast_ref::<Self>()
|
||||
.and_then(Self::http_error_details),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn from_box_error(mut source: Box<dyn std::error::Error + Send + Sync>) -> Self {
|
||||
source = match source.downcast::<Self>() {
|
||||
Ok(e) => match *e {
|
||||
|
||||
@@ -10,7 +10,7 @@ use reqwest::{
|
||||
use std::{collections::HashMap, future::Future, str::FromStr, sync::Arc, time::Duration};
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use crate::error::{Error, Result};
|
||||
use crate::error::{Error, HttpErrorSource, Result};
|
||||
use crate::remote::db::RemoteOptions;
|
||||
use crate::remote::retry::{ResolvedRetryConfig, RetryCounter};
|
||||
use crate::utils::validate_database_name;
|
||||
@@ -957,6 +957,11 @@ impl<S: HttpSend> RestfulLanceDbClient<S> {
|
||||
.check_response(&retry_counter.request_id, response)
|
||||
.await
|
||||
.unwrap_err();
|
||||
// A structured resource error is a domain result, not a
|
||||
// transient failure. Retrying it would hide the typed error.
|
||||
if source.http_error_details().is_some() {
|
||||
return Err(source);
|
||||
}
|
||||
retry_counter.increment_request_failures(source)?;
|
||||
}
|
||||
Err(err) if err.is_connect() => {
|
||||
@@ -1033,13 +1038,15 @@ impl<S: HttpSend> RestfulLanceDbClient<S> {
|
||||
Ok(response)
|
||||
} else {
|
||||
let response_text = response.text().await.ok();
|
||||
let message = if let Some(response_text) = response_text {
|
||||
format!("{}: {}", status, response_text)
|
||||
} else {
|
||||
status.to_string()
|
||||
let source = match response_text {
|
||||
Some(response_text) => HttpErrorSource::new(
|
||||
format!("{}: {}", status, response_text),
|
||||
Some(&response_text),
|
||||
),
|
||||
None => HttpErrorSource::new(status.to_string(), None),
|
||||
};
|
||||
Err(Error::Http {
|
||||
source: message.into(),
|
||||
source: Box::new(source),
|
||||
request_id: request_id.into(),
|
||||
status_code: Some(status),
|
||||
})
|
||||
@@ -1215,6 +1222,120 @@ pub mod test_utils {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#[tokio::test]
|
||||
async fn check_response_extracts_structured_resource_error() {
|
||||
use crate::remote::client::test_utils::client_with_handler;
|
||||
|
||||
let client =
|
||||
client_with_handler(|_| http::Response::builder().status(404).body("").unwrap());
|
||||
let cases = [
|
||||
(404, "not_found", "secret", "openai"),
|
||||
(404, "not_found", "function", "embed"),
|
||||
(404, "not_found", "namespace", "child"),
|
||||
(409, "already_exists", "namespace", "analytics"),
|
||||
];
|
||||
|
||||
for (status, error_code, resource_kind, resource_name) in cases {
|
||||
let body = serde_json::json!({
|
||||
"code": if error_code == "not_found" { 5 } else { 6 },
|
||||
"error": format!("{resource_kind} {resource_name} error"),
|
||||
"error_code": error_code,
|
||||
"resource": {
|
||||
"kind": resource_kind,
|
||||
"name": resource_name,
|
||||
"namespace_path": ["analytics"],
|
||||
},
|
||||
})
|
||||
.to_string();
|
||||
let response = http::Response::builder()
|
||||
.status(status)
|
||||
.body(body.clone())
|
||||
.unwrap()
|
||||
.into();
|
||||
|
||||
let error = client
|
||||
.check_response("request-1", response)
|
||||
.await
|
||||
.unwrap_err();
|
||||
let details = error
|
||||
.http_error_details()
|
||||
.expect("structured HTTP error details");
|
||||
assert_eq!(details.error_code, error_code);
|
||||
assert_eq!(details.resource_kind, resource_kind);
|
||||
assert_eq!(details.resource_name, resource_name);
|
||||
assert_eq!(details.namespace_path, ["analytics"]);
|
||||
assert!(error.to_string().contains(&body));
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn check_response_keeps_legacy_body_as_generic_http_error() {
|
||||
use crate::remote::client::test_utils::client_with_handler;
|
||||
|
||||
let client =
|
||||
client_with_handler(|_| http::Response::builder().status(404).body("").unwrap());
|
||||
let body = serde_json::json!({
|
||||
"code": 4,
|
||||
"error": "Not found: Secret \"openai\" does not exist",
|
||||
})
|
||||
.to_string();
|
||||
let response = http::Response::builder()
|
||||
.status(404)
|
||||
.body(body.clone())
|
||||
.unwrap()
|
||||
.into();
|
||||
|
||||
let error = client
|
||||
.check_response("request-1", response)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(error.http_error_details().is_none());
|
||||
assert!(error.to_string().contains(&body));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn send_with_retry_does_not_retry_structured_resource_error() {
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
|
||||
use crate::remote::client::test_utils::client_with_handler;
|
||||
|
||||
let attempts = Arc::new(AtomicUsize::new(0));
|
||||
let handler_attempts = Arc::clone(&attempts);
|
||||
let client = client_with_handler(move |_| {
|
||||
handler_attempts.fetch_add(1, Ordering::Relaxed);
|
||||
http::Response::builder()
|
||||
.status(409)
|
||||
.body(
|
||||
serde_json::json!({
|
||||
"code": 6,
|
||||
"error": "already exists",
|
||||
"error_code": "already_exists",
|
||||
"resource": {
|
||||
"kind": "namespace",
|
||||
"name": "analytics",
|
||||
"namespace_path": [],
|
||||
},
|
||||
})
|
||||
.to_string(),
|
||||
)
|
||||
.unwrap()
|
||||
});
|
||||
|
||||
let request = client.client.post("http://localhost/");
|
||||
let error = client
|
||||
.send_with_retry(request, None, true)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(attempts.load(Ordering::Relaxed), 1);
|
||||
let details = error
|
||||
.http_error_details()
|
||||
.expect("structured HTTP error details");
|
||||
assert_eq!(details.error_code, "already_exists");
|
||||
assert_eq!(details.resource_kind, "namespace");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_http_timeout_error_is_actionable() {
|
||||
use super::*;
|
||||
|
||||
Reference in New Issue
Block a user