api errors: determine success by http status, handle filter error

Author: yagebu

frontend/src/lib/fetch.ts

@@ -22,33 +22,20 @@ export async function fetch(
  * Handles JSON content for a Promise returned by fetch, also handling an HTTP
  * error status.
  */
-async function handleJSON(response: Response): Promise<unknown> {
+async function handleJSON(
+  response: Response,
+): Promise<Record<string, unknown>> {
+  const data: unknown = await response.json().catch(() => null);
   if (!response.ok) {
-    try {
-      const data: unknown = await response.json();
-      if (isJsonObject(data)) {
-        throw new FetchError(
-          typeof data.error === "string"
-            ? data.error
-            : "Invalid response: missing error",
-        );
-      }
-      throw new FetchError(response.statusText);
-    } catch {
-      throw new FetchError(response.statusText);
-    }
-  }
-  const data: unknown = await response.json();
-  if (!isJsonObject(data)) {
-    throw new FetchError("Invalid response: not an object");
-  }
-  if (data.success !== true) {
     throw new FetchError(
-      typeof data.error === "string"
+      isJsonObject(data) && typeof data.error === "string"
         ? data.error
-        : "Invalid response: missing error",
+        : response.statusText,
     );
   }
+  if (!isJsonObject(data)) {
+    throw new FetchError("Invalid response: not a valid JSON object");
+  }
   return data;
 }
 
@@ -79,8 +66,8 @@ export async function fetchJSON(
  */
 export async function handleText(response: Response): Promise<string> {
   if (!response.ok) {
-    const msg = await response.text();
-    throw new Error(msg || response.statusText);
+    const msg = await response.text().catch(() => response.statusText);
+    throw new FetchError(msg);
   }
   return response.text();
 }

src/fava/json_api.py

@@ -30,6 +30,7 @@
 from fava.context import g
 from fava.core.documents import filepath_in_document_folder
 from fava.core.documents import is_document_or_import_file
+from fava.core.filters import FilterError
 from fava.core.ingest import filepath_in_primary_imports_folder
 from fava.core.misc import align
 from fava.helpers import FavaAPIError
@@ -81,15 +82,15 @@ def __init__(self, param: str, expected: type) -> None:
 
 def json_err(msg: str, status: HTTPStatus) -> Response:
     """Jsonify the error message."""
-    res = jsonify({"success": False, "error": msg})
+    res = jsonify({"error": msg})
     res.status = status  # type: ignore[assignment]
     return res
 
 
 def json_success(data: Any) -> Response:
     """Jsonify the response."""
     return jsonify(
-        {"success": True, "data": data, "mtime": str(g.ledger.mtime)},
+        {"data": data, "mtime": str(g.ledger.mtime)},
     )
 
 
@@ -157,24 +158,29 @@ def __init__(self, filename: str) -> None:
 
 
 @json_api.errorhandler(FavaAPIError)
-def _json_api_fava_api_error(error: FavaAPIError) -> Response:
+def _(error: FavaAPIError) -> Response:
     log.error("Encountered FavaAPIError.", exc_info=error)
     return json_err(error.message, HTTPStatus.INTERNAL_SERVER_ERROR)
 
 
 @json_api.errorhandler(FavaJSONAPIError)
-def _json_api_fava_json_api(error: FavaJSONAPIError) -> Response:
+def _(error: FavaJSONAPIError) -> Response:
     return json_err(error.message, error.status)
 
 
+@json_api.errorhandler(FilterError)
+def _(error: FilterError) -> Response:
+    return json_err(error.message, HTTPStatus.BAD_REQUEST)
+
+
 @json_api.errorhandler(OSError)
-def _json_api_oserror(error: OSError) -> Response:  # pragma: no cover
+def _(error: OSError) -> Response:  # pragma: no cover
     log.error("Encountered OSError.", exc_info=error)
     return json_err(error.strerror, HTTPStatus.INTERNAL_SERVER_ERROR)
 
 
 @json_api.errorhandler(ValidationError)
-def _json_api_validation_error(error: ValidationError) -> Response:
+def _(error: ValidationError) -> Response:
     return json_err(f"Invalid API request: {error!s}", HTTPStatus.BAD_REQUEST)
 
 

tests/test_json_api.py

@@ -52,7 +52,6 @@ def assert_api_error(
     """Asserts that the response errored and contains the message."""
     assert response.status_code == status.value
     assert response.json
-    assert not response.json["success"], response.json
     err_msg = response.json["error"]
     assert isinstance(err_msg, str)
     if msg:
@@ -64,7 +63,6 @@ def assert_api_success(response: TestResponse, data: Any | None = None) -> Any:
     """Asserts that the request was successful and contains the data."""
     assert response.status_code == HTTPStatus.OK.value
     assert response.json
-    assert response.json["success"], response.json
     if data is not None:
         assert data == response.json["data"]
     return response.json["data"]
@@ -654,6 +652,15 @@ def test_api_commodities_empty(
     assert not data
 
 
+def test_api_filter_error(
+    test_client: FlaskClient,
+) -> None:
+    response = test_client.get(
+        "/long-example/api/commodities?time=20",
+    )
+    assert_api_error(response, status=HTTPStatus.BAD_REQUEST)
+
+
 @pytest.mark.parametrize(
     ("name", "url"),
     [