diff --git a/crates/fmp-py/README.md b/crates/fmp-py/README.md index c7734b59..958eae65 100644 --- a/crates/fmp-py/README.md +++ b/crates/fmp-py/README.md @@ -146,7 +146,7 @@ package root): | `FmpConfigError` | the client cannot be built (missing key, bad URL, insecure auth) | | `FmpValidationError` | an argument is rejected before the request | | `FmpTransportError` | the request never produced a response | -| `FmpStatusError` | the provider answered with a non-success status | +| `FmpStatusError` | the provider answered with a non-success status, or with a 200 whose body is its own error message (`Invalid name`, `{"Error Message": ...}`) | | `FmpDecodeError` | the body could not be decoded into the documented shape; `decode_path` names the member (for example `[37].beta`) and `decode_kind` the reason, never the value | ```python diff --git a/crates/fmp-py/python/fmp/_native/errors/__init__.pyi b/crates/fmp-py/python/fmp/_native/errors/__init__.pyi index d7e50af1..bf31679f 100644 --- a/crates/fmp-py/python/fmp/_native/errors/__init__.pyi +++ b/crates/fmp-py/python/fmp/_native/errors/__init__.pyi @@ -79,7 +79,7 @@ class FmpError(builtins.Exception): class FmpStatusError(FmpError): r""" - The provider returned a non-success HTTP status. + The provider returned a non-success HTTP status, or a success status whose body is its own error message. """ ... diff --git a/crates/fmp-py/src/errors.rs b/crates/fmp-py/src/errors.rs index c16b542d..6ab1b92f 100644 --- a/crates/fmp-py/src/errors.rs +++ b/crates/fmp-py/src/errors.rs @@ -141,7 +141,7 @@ fmp_exception!( fmp_exception!( FmpStatusError, FmpError, - "The provider returned a non-success HTTP status." + "The provider returned a non-success HTTP status, or a success status whose body is its own error message." ); fmp_exception!( FmpDecodeError, diff --git a/crates/fmp-py/tests/test_calendar_b.py b/crates/fmp-py/tests/test_calendar_b.py index 81d28e9f..4ddae0ef 100644 --- a/crates/fmp-py/tests/test_calendar_b.py +++ b/crates/fmp-py/tests/test_calendar_b.py @@ -217,6 +217,6 @@ def test_status_error_carries_the_calendar_endpoint_id( def test_decode_error_on_a_non_json_body(client: Any, fixture_server: FixtureServer, errors: SimpleNamespace) -> None: """A body that is not JSON maps to ``FmpDecodeError`` rather than a bare exception.""" - fixture_server.route("/dividends", b"not-json") + fixture_server.route("/dividends", b"not-json " * 40) with pytest.raises(errors.FmpDecodeError): client.calendar.dividends("AAPL") diff --git a/crates/fmp-py/tests/test_client.py b/crates/fmp-py/tests/test_client.py index f3953007..4cd10a1a 100644 --- a/crates/fmp-py/tests/test_client.py +++ b/crates/fmp-py/tests/test_client.py @@ -260,14 +260,14 @@ def test_status_error_is_structured(client: Any, fixture_server: FixtureServer, def test_decode_error_is_structured(client: Any, fixture_server: FixtureServer, errors: SimpleNamespace) -> None: """A 200 with an undecodable body raises ``FmpDecodeError``.""" - fixture_server.route(QUOTE_SHORT_PATH, b"not-json") + fixture_server.route(QUOTE_SHORT_PATH, b"not-json " * 40) with pytest.raises(errors.FmpDecodeError) as raised: client.quote.short("INVALID_JSON") error = raised.value assert error.category == "decode" assert error.endpoint == "quote-short" assert error.status == 200 - assert error.body == "not-json" + assert error.body == "not-json " * 40 assert error.body_truncated is False diff --git a/crates/fmp-py/tests/test_economics.py b/crates/fmp-py/tests/test_economics.py index 82e07778..149aea31 100644 --- a/crates/fmp-py/tests/test_economics.py +++ b/crates/fmp-py/tests/test_economics.py @@ -349,3 +349,27 @@ def test_decode_error_names_the_risk_premium_endpoint( with pytest.raises(errors.FmpDecodeError) as raised: client.economics.market_risk_premium() assert raised.value.endpoint == "market-risk-premium" + + +ERROR_MESSAGE_BODY = ( + '{\n "Error Message": "No Data for this symbol or invalid API call. ' + 'Please retry or visit our documentation at https://financialmodelingprep.com/developer/docs."\n}' +) + + +@pytest.mark.parametrize("body", ["Invalid name", ERROR_MESSAGE_BODY], ids=["plain-text", "error-message-object"]) +def test_provider_message_with_a_success_status_is_a_status_error( + client: Any, fixture_server: FixtureServer, errors: SimpleNamespace, body: str +) -> None: + """A 200 whose body is FMP's own error message raises ``FmpStatusError``, not ``FmpDecodeError``.""" + fixture_server.route("/economic-indicators", body.encode()) + with pytest.raises(errors.FmpStatusError) as raised: + client.economics.indicators("notARealIndicator") + error = raised.value + assert not isinstance(error, errors.FmpDecodeError) + assert error.category == "status" + assert error.endpoint == "economic-indicators" + assert error.status == 200 + assert error.body == body + assert error.decode_kind is None + assert str(error).startswith("provider returned an error message with HTTP status 200") diff --git a/crates/fmp-py/tests/test_funds_b.py b/crates/fmp-py/tests/test_funds_b.py index 98e796ef..062094b7 100644 --- a/crates/fmp-py/tests/test_funds_b.py +++ b/crates/fmp-py/tests/test_funds_b.py @@ -261,7 +261,7 @@ def test_status_error_carries_the_funds_endpoint_id( def test_non_json_body_is_a_decode_error(client: Any, fixture_server: FixtureServer, errors: SimpleNamespace) -> None: """A non-JSON body on the ETF path maps to ``FmpDecodeError``.""" - fixture_server.route("/etf/holdings", b"not-json") + fixture_server.route("/etf/holdings", b"not-json " * 40) with pytest.raises(errors.FmpDecodeError): client.funds.etf_holdings("SPY") diff --git a/crates/fmp-py/tests/test_news_b.py b/crates/fmp-py/tests/test_news_b.py index cad27fd5..87299b67 100644 --- a/crates/fmp-py/tests/test_news_b.py +++ b/crates/fmp-py/tests/test_news_b.py @@ -234,6 +234,6 @@ def test_status_error_carries_the_news_endpoint_id( def test_non_json_body_is_a_decode_error(client: Any, fixture_server: FixtureServer, errors: SimpleNamespace) -> None: """A non-JSON body on the authored feed maps to ``FmpDecodeError``.""" - fixture_server.route("/fmp-articles", b"not-json") + fixture_server.route("/fmp-articles", b"not-json " * 40) with pytest.raises(errors.FmpDecodeError): client.news.articles() diff --git a/crates/fmp-py/tests/test_screener.py b/crates/fmp-py/tests/test_screener.py index a8e5e3fc..be1059d9 100644 --- a/crates/fmp-py/tests/test_screener.py +++ b/crates/fmp-py/tests/test_screener.py @@ -212,7 +212,7 @@ def test_status_error_carries_the_screener_endpoint_id( def test_decode_error_for_a_non_json_body(client: Any, fixture_server: FixtureServer, errors: SimpleNamespace) -> None: """A body that is not JSON maps to ``FmpDecodeError`` rather than a raw exception.""" - fixture_server.route("/company-screener", b"not-json") + fixture_server.route("/company-screener", b"not-json " * 40) with pytest.raises(errors.FmpDecodeError) as caught: client.screener.companies() diff --git a/crates/fmp-py/tests/test_sec_filings_b.py b/crates/fmp-py/tests/test_sec_filings_b.py index 77ac5be7..6b7e3cd7 100644 --- a/crates/fmp-py/tests/test_sec_filings_b.py +++ b/crates/fmp-py/tests/test_sec_filings_b.py @@ -298,7 +298,7 @@ def test_status_error_carries_the_sec_endpoint_id( def test_non_json_body_is_a_decode_error(client: Any, fixture_server: FixtureServer, errors: SimpleNamespace) -> None: """A non-JSON body on a lookup maps to ``FmpDecodeError``.""" - fixture_server.route("/sec-profile", b"not-json") + fixture_server.route("/sec-profile", b"not-json " * 40) with pytest.raises(errors.FmpDecodeError): client.sec_filings.company_profile("AAPL") diff --git a/crates/libfmp/src/client.rs b/crates/libfmp/src/client.rs index 44be24fc..314c047e 100644 --- a/crates/libfmp/src/client.rs +++ b/crates/libfmp/src/client.rs @@ -22,7 +22,7 @@ use url::Url; use crate::{ Result, VERSION, config::{Authentication, DEFAULT_BASE_URL, DEFAULT_PATH_PREFIX, RedirectPolicy}, - endpoints::{QueryEncoder, ResponseMetadata}, + endpoints::{DecodeFailure, QueryEncoder, ResponseMetadata}, error::{ConfigurationErrorKind, Error, Redactor, SafeBody, SecretString}, transport::{HttpExecutor, PreparedRequest, ReqwestExecutor, TransportResponse}, }; @@ -489,17 +489,19 @@ impl Client { response.body_bytes(), ResponseMetadata::new(content_type, content_disposition), ) - .map_err(|failure| { - Error::decode( + .map_err(|failure| match failure { + DecodeFailure::Shape { path, kind } => Error::decode( Some(endpoint.id()), Some(response.status()), Some(safe_body(response.body(), redactor)), "successful response could not be decoded", ) - .with_decode_location( - failure.path.map(|path| redactor.redact(&path)), - failure.kind, - ) + .with_decode_location(path.map(|path| redactor.redact(&path)), kind), + DecodeFailure::ProviderMessage => Error::provider_message( + endpoint.id(), + response.status(), + Some(safe_body(response.body(), redactor)), + ), }); } diff --git a/crates/libfmp/src/endpoints/economics.rs b/crates/libfmp/src/endpoints/economics.rs index 82891768..3925b25a 100644 --- a/crates/libfmp/src/endpoints/economics.rs +++ b/crates/libfmp/src/endpoints/economics.rs @@ -149,6 +149,10 @@ impl Client { /// The documented 90-day maximum is advisory metadata. Use /// [`EndpointBounds::accepts_date_range`](super::metadata::EndpointBounds::accepts_date_range) /// for optional preflight validation. + /// + /// The provider also caps the rows returned per call (about 60 observed + /// live), silently dropping the rest of a longer window. Walk long ranges + /// in short windows and merge the pages. pub async fn treasury_rates( &self, query: impl Into, @@ -161,6 +165,20 @@ impl Client { /// The documented 90-day maximum is advisory metadata. Use /// [`EndpointBounds::accepts_date_range`](super::metadata::EndpointBounds::accepts_date_range) /// for optional preflight validation. + /// + /// The provider caps the rows returned per call and can return fewer rows + /// than the window holds, or none: observed live, a one-year CPI window + /// returned 2 rows, a 91-day window 3, and a six-year GDP window `[]`. + /// Walk long ranges in short windows and merge the pages. + /// + /// # Errors + /// + /// An unknown indicator name is answered with HTTP 200 and the plain-text + /// body `Invalid name`; it surfaces as an [`ErrorCategory::Status`] + /// error whose [`Error::status_code`] is `Some(200)`, not a decode error. + /// + /// [`ErrorCategory::Status`]: crate::error::ErrorCategory::Status + /// [`Error::status_code`]: crate::Error::status_code pub async fn economic_indicators( &self, query: impl Into, diff --git a/crates/libfmp/src/endpoints/mod.rs b/crates/libfmp/src/endpoints/mod.rs index fa67cca0..a0364718 100644 --- a/crates/libfmp/src/endpoints/mod.rs +++ b/crates/libfmp/src/endpoints/mod.rs @@ -33,10 +33,10 @@ pub mod technical_indicators; pub mod tipranks; pub mod transcripts; -use std::{fmt, marker::PhantomData}; +use std::{collections::BTreeMap, fmt, marker::PhantomData}; use bytes::Bytes; -use serde::de::DeserializeOwned; +use serde::de::{DeserializeOwned, IgnoredAny}; use crate::{error::DecodeErrorKind, transport::HttpMethod}; use metadata::EndpointMetadata; @@ -335,14 +335,20 @@ impl<'a> ResponseMetadata<'a> { } } -/// Where and why a response body failed to decode. -/// -/// Only the member path and a coarse kind survive: the decoder's own message -/// can quote the offending value, so it is classified and then dropped. +/// Why a successful response body did not yield the documented value. #[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct DecodeFailure { - pub(crate) path: Option, - pub(crate) kind: DecodeErrorKind, +pub(crate) enum DecodeFailure { + /// The body does not match the documented shape. + /// + /// Only the member path and a coarse kind survive: the decoder's own + /// message can quote the offending value, so it is classified and then + /// dropped. + Shape { + path: Option, + kind: DecodeErrorKind, + }, + /// The provider answered a success status with its own error message. + ProviderMessage, } type Decoder = for<'a> fn(Bytes, ResponseMetadata<'a>) -> std::result::Result; @@ -411,16 +417,49 @@ fn decode_json( where R: DeserializeOwned, { + if is_provider_message(&body) { + return Err(DecodeFailure::ProviderMessage); + } let mut deserializer = serde_json::Deserializer::from_slice(&body); let value = serde_path_to_error::deserialize(&mut deserializer) .map_err(|error| decode_failure(error.path(), error.inner()))?; - deserializer.end().map_err(|_| DecodeFailure { + deserializer.end().map_err(|_| DecodeFailure::Shape { path: None, kind: DecodeErrorKind::Syntax, })?; Ok(value) } +const MAX_PLAIN_TEXT_MESSAGE_BYTES: usize = 256; +const MAX_ERROR_MESSAGE_OBJECT_BYTES: usize = 1024; +const ERROR_MESSAGE_MEMBER: &str = "Error Message"; + +/// Recognizes the two error shapes FMP sends with HTTP 200. +/// +/// One is a short plain-text line such as `Invalid name` that is not JSON at +/// all; the other is a JSON object whose only member is `"Error Message"`. +/// Both checks are bounded by size so ordinary payloads are not parsed twice. +fn is_provider_message(body: &[u8]) -> bool { + let body = body.trim_ascii(); + match body.first() { + None => false, + Some(b'{') => { + body.len() <= MAX_ERROR_MESSAGE_OBJECT_BYTES + && serde_json::from_slice::>(body).is_ok_and( + |members| members.len() == 1 && members.contains_key(ERROR_MESSAGE_MEMBER), + ) + } + Some(b'[') => false, + Some(_) => { + body.len() <= MAX_PLAIN_TEXT_MESSAGE_BYTES + && body + .iter() + .all(|byte| byte.is_ascii_graphic() || *byte == b' ') + && serde_json::from_slice::(body).is_err() + } + } +} + fn decode_failure(path: &serde_path_to_error::Path, error: &serde_json::Error) -> DecodeFailure { let message = error.to_string(); let kind = match error.classify() { @@ -436,7 +475,7 @@ fn decode_failure(path: &serde_path_to_error::Path, error: &serde_json::Error) - None => member.to_owned(), }); } - DecodeFailure { path, kind } + DecodeFailure::Shape { path, kind } } fn data_error_kind(message: &str) -> DecodeErrorKind { diff --git a/crates/libfmp/src/error.rs b/crates/libfmp/src/error.rs index 5a378b0f..b15eca99 100644 --- a/crates/libfmp/src/error.rs +++ b/crates/libfmp/src/error.rs @@ -45,7 +45,8 @@ pub enum ErrorCategory { Configuration, /// A request could not be completed by the transport. Transport, - /// The provider returned a non-success HTTP status. + /// The provider returned a non-success HTTP status, or answered a success + /// status with its own error message instead of the documented payload. Status, /// A successful response could not be decoded. Decode, @@ -554,6 +555,28 @@ impl Error { error } + /// Creates an error for a success status whose body is a provider error + /// message, such as FMP's plain-text `Invalid name` or a JSON object whose + /// only member is `"Error Message"`. + /// + /// The category is [`ErrorCategory::Status`] and the status is the + /// success code the provider sent. `endpoint` is a static logical identity + /// or relative path, never a URL. + pub(crate) fn provider_message( + endpoint: &'static str, + status: u16, + body: Option, + ) -> Self { + let mut error = Self::new( + ErrorCategory::Status, + format!("provider returned an error message with HTTP status {status}"), + ) + .with_endpoint(Some(endpoint)); + error.status = Some(status); + error.body = body; + error + } + /// Creates a response decoding error with available response context. /// /// `endpoint` is a static logical identity or relative path, never a URL, diff --git a/crates/libfmp/tests/decode_path.rs b/crates/libfmp/tests/decode_path.rs index 2bdd1501..8608dfb3 100644 --- a/crates/libfmp/tests/decode_path.rs +++ b/crates/libfmp/tests/decode_path.rs @@ -159,8 +159,8 @@ async fn malformed_and_trailing_json_are_syntax_failures() { #[tokio::test] async fn non_json_and_empty_bodies_are_root_syntax_failures() { - for body in ["not-json", ""] { - let error = decode_error(json_body(body.to_owned())).await; + for body in ["not-json ".repeat(40), String::new()] { + let error = decode_error(json_body(body)).await; assert_eq!(error.decode_kind(), Some(DecodeErrorKind::Syntax)); assert_eq!(error.decode_path(), None); diff --git a/crates/libfmp/tests/provider_message.rs b/crates/libfmp/tests/provider_message.rs new file mode 100644 index 00000000..b510eaf3 --- /dev/null +++ b/crates/libfmp/tests/provider_message.rs @@ -0,0 +1,115 @@ +mod support; + +use std::sync::Arc; + +use libfmp::{ + Client, + config::Authentication, + endpoints::economics::EconomicIndicatorsQuery, + error::{DecodeErrorKind, ErrorCategory}, + http::{HeaderMap, HeaderValue, header::CONTENT_TYPE}, + query::EconomicIndicator, + transport::TransportResponse, +}; + +use support::{FixtureExecutor, json_fixture}; + +const INDICATORS: &[u8] = include_bytes!("fixtures/economic_indicators.json"); +const ERROR_MESSAGE: &str = "{\n \"Error Message\": \"No Data for this symbol or invalid API call. \ + Please retry or visit our documentation at https://financialmodelingprep.com/developer/docs.\"\n}"; + +fn json_body(body: &str) -> TransportResponse { + let mut headers = HeaderMap::new(); + headers.insert( + CONTENT_TYPE, + HeaderValue::from_static("application/json; charset=utf-8"), + ); + TransportResponse::new(200, headers, body.to_owned()) +} + +async fn economic_indicators( + response: TransportResponse, +) -> libfmp::Result> { + let executor = Arc::new(FixtureExecutor::new([response])); + let client = Client::builder() + .base_url("https://proxy.example/router") + .path_prefix("stable") + .authentication(Authentication::None) + .executor(executor.clone()) + .build() + .unwrap(); + let result = client + .economic_indicators(EconomicIndicatorsQuery::new(EconomicIndicator::Gdp)) + .await; + assert_eq!(executor.requests().len(), 1); + result +} + +fn assert_provider_message(error: &libfmp::Error, body: &str) { + assert_eq!(error.category(), ErrorCategory::Status); + assert_eq!(error.status_code(), Some(200)); + assert_eq!(error.endpoint(), Some("economic-indicators")); + assert_eq!( + error.message(), + "provider returned an error message with HTTP status 200" + ); + assert_eq!(error.decode_kind(), None); + assert_eq!(error.decode_path(), None); + assert_eq!(error.body().unwrap().as_str(), body); +} + +#[tokio::test] +async fn plain_text_success_body_is_a_provider_message() { + let error = economic_indicators(json_body("Invalid name")) + .await + .unwrap_err(); + + assert_provider_message(&error, "Invalid name"); + assert_eq!( + error.to_string(), + "provider returned an error message with HTTP status 200 \ + (endpoint: economic-indicators): Invalid name" + ); +} + +#[tokio::test] +async fn error_message_object_is_a_provider_message() { + let error = economic_indicators(json_body(ERROR_MESSAGE)) + .await + .unwrap_err(); + + assert_provider_message(&error, ERROR_MESSAGE); +} + +#[tokio::test] +async fn documented_rows_still_decode() { + let rows = economic_indicators(json_fixture(INDICATORS)).await.unwrap(); + + assert!(!rows.is_empty()); +} + +#[tokio::test] +async fn other_malformed_bodies_stay_decode_errors() { + for body in [ + "", + "null", + "42", + "{\"Error Message\": \"x\", \"date\": \"2024-01-01\"}", + "[\"Invalid name\"]", + "Invalid\u{7}name", + ] { + let error = economic_indicators(json_body(body)).await.unwrap_err(); + + assert_eq!(error.category(), ErrorCategory::Decode, "{body:?}"); + assert!(error.decode_kind().is_some(), "{body:?}"); + } +} + +#[tokio::test] +async fn long_plain_text_body_stays_a_syntax_error() { + let body = "x ".repeat(200); + let error = economic_indicators(json_body(&body)).await.unwrap_err(); + + assert_eq!(error.category(), ErrorCategory::Decode); + assert_eq!(error.decode_kind(), Some(DecodeErrorKind::Syntax)); +} diff --git a/crates/libfmp/tests/transport.rs b/crates/libfmp/tests/transport.rs index c5c597ea..9ba54d66 100644 --- a/crates/libfmp/tests/transport.rs +++ b/crates/libfmp/tests/transport.rs @@ -823,7 +823,7 @@ async fn overlapping_secrets_are_redacted_from_status_and_decode_bodies() { Ok(TransportResponse::new( 200, json_headers, - "shared-secret-prefix-and-suffix", + "{shared-secret-prefix-and-suffix}", )), ])); let client = Client::builder() @@ -834,10 +834,13 @@ async fn overlapping_secrets_are_redacted_from_status_and_decode_bodies() { .build() .unwrap(); - for expected_category in [ErrorCategory::Status, ErrorCategory::Decode] { + for (expected_category, expected_body) in [ + (ErrorCategory::Status, "[REDACTED]"), + (ErrorCategory::Decode, "{[REDACTED]}"), + ] { let error = client.execute(&endpoint()).await.unwrap_err(); assert_eq!(error.category(), expected_category); - assert_eq!(error.body().map(|body| body.as_str()), Some("[REDACTED]")); + assert_eq!(error.body().map(|body| body.as_str()), Some(expected_body)); let diagnostic = format!("{error:?} {error}"); assert!(!diagnostic.contains("shared-secret-prefix")); assert!(!diagnostic.contains("and-suffix")); diff --git a/docs/contract-ambiguities.md b/docs/contract-ambiguities.md index 995da8a0..7b6e82a9 100644 --- a/docs/contract-ambiguities.md +++ b/docs/contract-ambiguities.md @@ -31,7 +31,8 @@ each row; `git log -S` on the quoted key finds the current location. | Crowdfunding search | `date` documented only as null (Regulation D search proves a datetime). | Required-present `Option`. | `crates/libfmp/src/responses/fundraising.rs:31-32`; ADR 0027 lines 29-33 | | SEC company profile | `securityType` documented only as null. | Nullable `Option`. | `crates/libfmp/src/responses/sec_filings.rs:80-81`; ADR 0019 line 45 | | Symbol-change directory | `invalid` parameter is a lowercase string with undocumented semantics. | `TrueFalseFlag` passed through uninterpreted. | `crates/libfmp/src/endpoints/directory.rs:104-118` | -| Economic indicators | Example range spans a year while the note caps ranges at 90 days. | Both dates preserved; 90-day bound exposed as metadata, not enforced. | ADR 0010 lines 28-33 | +| Economic indicators | Example range spans a year while the note caps ranges at 90 days. Live (2026-09-30), rows per call are capped below the window: a one-year CPI window returned 2 rows, a 91-day window 3, and a six-year GDP window `[]`. An unknown `name` is answered with HTTP 200 and the plain-text body `Invalid name` (#381). | Both dates preserved; 90-day bound exposed as metadata, not enforced. Callers walk short windows. A 200 plain-text or sole-`"Error Message"` body is a Status error with status 200, not a decode error. | `crates/libfmp/src/endpoints/economics.rs`; `crates/libfmp/src/endpoints/mod.rs` `is_provider_message`; ADR 0010 lines 28-33 | +| Treasury rates | Live, a call returns at most about 60 rows regardless of the requested window. | 90-day bound exposed as metadata, not enforced; callers walk short windows. | `crates/libfmp/src/endpoints/economics.rs` | | Market risk premium | Prose mentions dates; no parameters are documented. | Queryless descriptor; date parameters deferred. | ADR 0010 lines 35-38 | | As-reported, segmentation, financial reports | Issuer taxonomy keys and value kinds are open-ended. | `DynamicObject` payload; duplicate JSON members collapse to the last value. | `crates/libfmp/src/responses/statements/{as_reported,segmentation,reports}.rs`; ADR 0008 lines 26-35 | | Float decode fidelity | Two documented fixtures decode 1 ULP away from Python's `json` (`altmanZScore`, `growthCashAtEndOfPeriod`); serde_json's default float parser is not correctly rounded. | Python tests compare floats with `pytest.approx`. Candidate fix: `float_roundtrip` feature plus a decode test against the fixture literal. | Issue #40 comment (PY2-10, #176); `crates/fmp-py/tests/test_chart_a.py:8-9` | diff --git a/sdk/go/README.md b/sdk/go/README.md index f2b46a9c..1fbe49c0 100644 --- a/sdk/go/README.md +++ b/sdk/go/README.md @@ -163,7 +163,10 @@ if errors.As(err, &fmpErr) { `Category` is one of `Validation`, `Configuration`, `Transport`, `Status`, or `Decode`. `Endpoint` is a logical endpoint id, never a URL. `Body` is the -provider body, redacted and capped. A `Decode` error also carries `Path`, the +provider body, redacted and capped. A `Status` error can carry `Status` 200 +when FMP answers a success status with its own error message (the plain-text +`Invalid name`, or an object whose only member is `"Error Message"`) instead +of the documented payload. A `Decode` error also carries `Path`, the JSON pointer of the failing member (`/37/beta`), and `DecodeKind`, the coarse reason (`DecodeKindNull`, `DecodeKindWrongType`, ...); neither ever includes the member value. The client never retries, wraps every diff --git a/sdk/go/client.go b/sdk/go/client.go index d701d440..16f1fe6d 100644 --- a/sdk/go/client.go +++ b/sdk/go/client.go @@ -320,6 +320,9 @@ func (c *Client) getJSON(ctx context.Context, endpointID, relativePath string, q return decodeError(endpointID, resp.status, c.safeBody(resp.body), "successful response used an unexpected content type", nil) } + if isProviderMessage(resp.body) { + return providerMessageError(endpointID, resp.status, c.safeBody(resp.body)) + } if err := json.Unmarshal(resp.body, out); err != nil { decoded := decodeError(endpointID, resp.status, c.safeBody(resp.body), "successful response could not be decoded", c.redactCause(err)) diff --git a/sdk/go/decode.go b/sdk/go/decode.go index 4c0de85d..48844288 100644 --- a/sdk/go/decode.go +++ b/sdk/go/decode.go @@ -1,6 +1,7 @@ package fmp import ( + "bytes" "encoding/json/jsontext" "encoding/json/v2" "errors" @@ -134,6 +135,48 @@ func decodeLocation(body []byte, err error) (string, DecodeKind) { } } +const ( + maxPlainTextMessageBytes = 256 + maxErrorMessageObjectBytes = 1024 + errorMessageMember = "Error Message" +) + +// isProviderMessage recognizes the two error shapes FMP sends with HTTP 200, +// mirroring the Rust crate: a short plain-text line such as "Invalid name" +// that is not JSON at all, or a JSON object whose only member is +// "Error Message". Both checks are bounded by size so ordinary payloads are +// not parsed twice. +func isProviderMessage(body []byte) bool { + body = bytes.Trim(body, " \t\n\f\r") + if len(body) == 0 { + return false + } + switch body[0] { + case '{': + if len(body) > maxErrorMessageObjectBytes { + return false + } + var members map[string]jsontext.Value + if json.Unmarshal(body, &members) != nil { + return false + } + _, ok := members[errorMessageMember] + return ok && len(members) == 1 + case '[': + return false + default: + if len(body) > maxPlainTextMessageBytes { + return false + } + for _, b := range body { + if b < ' ' || b > '~' { + return false + } + } + return !jsontext.Value(body).IsValid() + } +} + // valueAt walks body along pointer one level at a time and reports the value // it names, or false when a token does not resolve. func valueAt(body []byte, pointer jsontext.Pointer) (jsontext.Value, bool) { diff --git a/sdk/go/decode_path_test.go b/sdk/go/decode_path_test.go index a325eeee..ee57f5e1 100644 --- a/sdk/go/decode_path_test.go +++ b/sdk/go/decode_path_test.go @@ -73,7 +73,7 @@ func TestDecodeErrorLocatesMissingWrongAndMalformedMembers(t *testing.T) { {"wrong type", strings.Replace(row, `"AAPL"`, "5", 1), "/0/symbol", DecodeKindWrongType}, {"root shape", "{}", "", DecodeKindWrongType}, {"malformed", `[{"beta": nope}]`, "/0/beta", DecodeKindSyntax}, - {"not json", "not-json", "", DecodeKindSyntax}, + {"not json", strings.Repeat("not-json ", 40), "", DecodeKindSyntax}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/sdk/go/errors.go b/sdk/go/errors.go index 1e5257ca..bb1ae209 100644 --- a/sdk/go/errors.go +++ b/sdk/go/errors.go @@ -29,7 +29,8 @@ const ( CategoryConfiguration // CategoryTransport reports a request the transport could not complete. CategoryTransport - // CategoryStatus reports a non-success HTTP status from the provider. + // CategoryStatus reports a non-success HTTP status from the provider, or a + // success status whose body is the provider's own error message. CategoryStatus // CategoryDecode reports a successful response that could not be decoded. CategoryDecode @@ -251,6 +252,19 @@ func statusError(endpoint string, status int, body *SafeBody) *Error { } } +// providerMessageError reports a success status whose body is a provider +// error message rather than the documented payload (see isProviderMessage). +// It is a CategoryStatus error carrying the success status the provider sent. +func providerMessageError(endpoint string, status int, body *SafeBody) *Error { + return &Error{ + Category: CategoryStatus, + Message: fmt.Sprintf("provider returned an error message with HTTP status %d", status), + Endpoint: endpoint, + Status: status, + Body: body, + } +} + func decodeError(endpoint string, status int, body *SafeBody, message string, cause error) *Error { return &Error{ Category: CategoryDecode, diff --git a/sdk/go/provider_message_test.go b/sdk/go/provider_message_test.go new file mode 100644 index 00000000..3b67c3cc --- /dev/null +++ b/sdk/go/provider_message_test.go @@ -0,0 +1,85 @@ +package fmp + +import ( + "context" + "net/http" + "strings" + "testing" +) + +const errorMessageBody = "{\n \"Error Message\": \"No Data for this symbol or invalid API call. " + + "Please retry or visit our documentation at https://financialmodelingprep.com/developer/docs.\"\n}" + +// economicIndicatorsBody serves body with HTTP 200 as the economic-indicators +// response and returns the rows and error of the call. +func economicIndicatorsBody(t *testing.T, body string) ([]EconomicIndicatorObservation, error) { + t.Helper() + server, _ := newServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + _, _ = w.Write([]byte(body)) + }) + client := newClient(t, server) + return client.Economics.Indicators(context.Background(), NewEconomicIndicatorsQuery(EconomicIndicatorGdp)) +} + +func TestSuccessStatusProviderMessagesAreStatusErrors(t *testing.T) { + t.Parallel() + cases := []struct { + name string + body string + }{ + {"plain text", "Invalid name"}, + {"error message object", errorMessageBody}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := economicIndicatorsBody(t, tc.body) + typed := assertQuoteError(t, err, CategoryStatus, http.StatusOK, "economic-indicators") + if typed.Message != "provider returned an error message with HTTP status 200" { + t.Fatalf("Message = %q", typed.Message) + } + if typed.DecodeKind != DecodeKindNone || typed.Path != "" { + t.Fatalf("DecodeKind = %v, Path = %q, want none", typed.DecodeKind, typed.Path) + } + if typed.Body == nil || typed.Body.Text != tc.body { + t.Fatalf("Body = %+v, want %q", typed.Body, tc.body) + } + }) + } +} + +func TestPlainTextProviderMessageFormatsLikeRust(t *testing.T) { + t.Parallel() + _, err := economicIndicatorsBody(t, "Invalid name") + want := "provider returned an error message with HTTP status 200 (endpoint: economic-indicators): Invalid name" + if err == nil || err.Error() != want { + t.Fatalf("Error() = %v, want %q", err, want) + } +} + +func TestDocumentedIndicatorRowsStillDecode(t *testing.T) { + t.Parallel() + rows, err := economicIndicatorsBody(t, string(readFixture(t, "economic_indicators.json"))) + if err != nil || len(rows) == 0 { + t.Fatalf("Indicators = %+v, %v", rows, err) + } +} + +func TestOtherMalformedBodiesStayDecodeErrors(t *testing.T) { + t.Parallel() + for _, body := range []string{ + "", + "42", + `{"Error Message": "x", "date": "2024-01-01"}`, + `["Invalid name"]`, + "Invalid\x07name", + strings.Repeat("x ", 200), + } { + _, err := economicIndicatorsBody(t, body) + typed := assertQuoteError(t, err, CategoryDecode, http.StatusOK, "economic-indicators") + if typed.DecodeKind == DecodeKindNone { + t.Fatalf("body %q: DecodeKind is none", body) + } + } +} diff --git a/sdk/go/transport_test.go b/sdk/go/transport_test.go index 606ddfd0..1667d576 100644 --- a/sdk/go/transport_test.go +++ b/sdk/go/transport_test.go @@ -237,7 +237,7 @@ func TestRedirectLimitAndRedirectNone(t *testing.T) { func TestResponseBodyCapIsEnforced(t *testing.T) { t.Parallel() - body := strings.Repeat("x", 64) + body := strings.Repeat("x", 300) for _, chunked := range []bool{false, true} { server, _ := newServer(t, func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json")