fix(probe): tell a body that never arrived from one that is not the resource

reqwest reports both as a decode error, so the transport step and the
decode step now fail with their own types: a truncated or timed out body
stays transient, while a body that is not the resource is a verdict about
the endpoint.

Also folds the status table into `classified`, so the absence meaning is
only ever read from the response type.

Author: haiyuechen-nearone

crates/foreign-chain-inspector/src/aptos/inspector.rs

@@ -100,7 +100,6 @@ where
     }
 }
 
-/// A transaction the chain does not have is a verdict about that transaction.
 impl HasAbsenceMeaning for TransactionResponse {
     const ABSENCE: AbsenceMeaning = AbsenceMeaning::TransactionIsAbsent;
 }
@@ -114,38 +113,45 @@ impl<T: HasAbsenceMeaning> ClassifyRpcOutcome for Result<T, AptosRpcError> {
     type Response = T;
 
     fn classified(self) -> Result<T, ForeignChainInspectionError> {
-        self.map_err(|error| classify_rest_error(error, T::ABSENCE))
-    }
-}
+        let error = match self {
+            Ok(response) => return Ok(response),
+            Err(error) => error,
+        };
 
-fn classify_rest_error(
-    error: AptosRpcError,
-    absence: AbsenceMeaning,
-) -> ForeignChainInspectionError {
-    let message = error.to_string();
-    match error {
-        // Aptos answers 404 both for a transaction it does not have and for a path it does not
-        // route; only the resource that was read tells the two apart.
-        AptosRpcError::ApiError { status: 404, .. } => match absence {
-            AbsenceMeaning::TransactionIsAbsent => ForeignChainInspectionError::TransactionNotFound,
-            AbsenceMeaning::ApiIsNotServed => {
+        let message = error.to_string();
+        Err(match error {
+            // Aptos answers 404 both for a transaction it lacks and for a path it does not route.
+            AptosRpcError::ApiError { status: 404, .. } => match T::ABSENCE {
+                AbsenceMeaning::TransactionIsAbsent => {
+                    ForeignChainInspectionError::TransactionNotFound
+                }
+                AbsenceMeaning::ApiIsNotServed => {
+                    ForeignChainInspectionError::RpcRequestRejected(message)
+                }
+            },
+            // Rate limits and server errors are provider hiccups → transient, so the
+            // affected provider is dropped from the quorum instead of blocking it.
+            AptosRpcError::ApiError {
+                status: 408 | 429, ..
+            } => ForeignChainInspectionError::RpcRequestFailed(message),
+            AptosRpcError::ApiError { status, .. } if status >= 500 => {
+                ForeignChainInspectionError::RpcRequestFailed(message)
+            }
+            // Remaining 4xx (400/401/403/410, …) are deterministic rejections —
+            // retrying cannot change them, so they count as substantive verdicts.
+            AptosRpcError::ApiError { .. } => {
                 ForeignChainInspectionError::RpcRequestRejected(message)
             }
-        },
-        // Rate limits and server errors are provider hiccups → transient, so the affected
-        // provider is dropped from the quorum instead of blocking it.
-        AptosRpcError::ApiError {
-            status: 408 | 429, ..
-        } => ForeignChainInspectionError::RpcRequestFailed(message),
-        AptosRpcError::ApiError { status, .. } if status >= 500 => {
-            ForeignChainInspectionError::RpcRequestFailed(message)
-        }
-        // Remaining 4xx (400/401/403/410, …) are deterministic rejections — retrying cannot
-        // change them, so they count as substantive verdicts.
-        AptosRpcError::ApiError { .. } => ForeignChainInspectionError::RpcRequestRejected(message),
-        // Named so a probe can report a slow provider as timed out rather than unreachable.
-        AptosRpcError::Http(error) if error.is_timeout() => ForeignChainInspectionError::Timeout,
-        AptosRpcError::Http(_) => ForeignChainInspectionError::RpcRequestFailed(message),
+            // Split timeout from rest of http errors for reporting.
+            AptosRpcError::Http(error) if error.is_timeout() => {
+                ForeignChainInspectionError::Timeout
+            }
+            AptosRpcError::Http(_) => ForeignChainInspectionError::RpcRequestFailed(message),
+            // A body that will not decode is not transient.
+            AptosRpcError::MalformedBody(_) => {
+                ForeignChainInspectionError::MalformedRpcResponse(message)
+            }
+        })
     }
 }
 
@@ -337,10 +343,7 @@ mod tests {
                     status: *status,
                     body: body.clone(),
                 }),
-                Err(other) => Err(AptosRpcError::ApiError {
-                    status: 500,
-                    body: other.to_string(),
-                }),
+                Err(other) => unreachable!("MockAptosClient models only ApiError, got {other}"),
             };
             std::future::ready(r)
         }
@@ -354,10 +357,7 @@ mod tests {
                     status: *status,
                     body: body.clone(),
                 }),
-                Err(other) => Err(AptosRpcError::ApiError {
-                    status: 500,
-                    body: other.to_string(),
-                }),
+                Err(other) => unreachable!("MockAptosClient models only ApiError, got {other}"),
             };
             std::future::ready(r)
         }
@@ -878,21 +878,20 @@ mod tests {
     #[case::internal_error(500)]
     #[case::bad_request(400)]
     #[case::unauthorized(401)]
-    fn classify_rest_error__should_read_every_status_but_404_alike(#[case] status: u16) {
+    fn classified__should_read_every_status_but_404_alike(#[case] status: u16) {
+        // Given
+        let read_as_transaction: Result<TransactionResponse, _> =
+            Err(MockAptosClient::error(status));
+        let read_as_api_root: Result<LedgerInfoResponse, _> = Err(MockAptosClient::error(status));
+
         // When
-        let as_transaction = classify_rest_error(
-            MockAptosClient::error(status),
-            AbsenceMeaning::TransactionIsAbsent,
-        );
-        let as_api_root = classify_rest_error(
-            MockAptosClient::error(status),
-            AbsenceMeaning::ApiIsNotServed,
-        );
+        let from_transaction = read_as_transaction.classified().unwrap_err();
+        let from_api_root = read_as_api_root.classified().unwrap_err();
 
         // Then
         assert_eq!(
-            std::mem::discriminant(&as_transaction),
-            std::mem::discriminant(&as_api_root)
+            std::mem::discriminant(&from_transaction),
+            std::mem::discriminant(&from_api_root)
         );
     }
 }

crates/foreign-chain-inspector/src/lib.rs

@@ -470,33 +470,25 @@ fn is_retryable_status(status_code: u16) -> bool {
     matches!(status_code, REQUEST_TIMEOUT | TOO_MANY_REQUESTS) || status_code >= SERVER_ERROR
 }
 
-/// What a provider's "not found" answer means for the resource that was read.
-///
-/// HTTP 404 and gRPC `NOT_FOUND` are the one wire condition whose verdict depends on what was
-/// asked for rather than on the status itself, so the resource answers and the chain's status
-/// table reads the answer.
+/// What a provider's "not found" answer means for the resource that was read: the one wire
+/// condition whose verdict depends on what was asked for rather than on the status itself.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) enum AbsenceMeaning {
-    /// The chain may legitimately not hold it, so absence is the chain's own verdict.
+    /// The chain may legitimately not hold it.
     TransactionIsAbsent,
-    /// Every node serving this chain's API holds it, so absence says the endpoint serves a
-    /// different API.
+    /// Every node serving this chain's API holds it.
     ApiIsNotServed,
 }
 
-/// The [`AbsenceMeaning`] of the resource a response carries, stated once beside that response
-/// rather than at each call that reads it.
-///
-/// [`ClassifyRpcOutcome::classified`] requires it, so a call reading a response that has no
-/// answer to the question does not compile.
+/// The [`AbsenceMeaning`] of the resource a response carries. Required by
+/// [`ClassifyRpcOutcome::classified`], so a response that never answered cannot be classified.
 pub(crate) trait HasAbsenceMeaning {
     const ABSENCE: AbsenceMeaning;
 }
 
-/// Reads a chain client's outcome as an inspection outcome: the transport says how the call
-/// failed, the response type says what an absent resource means, and the call site says nothing.
-///
-/// One implementation per transport, each holding that chain's whole status table.
+/// Reads a chain client's outcome as an inspection outcome: the response type supplies what
+/// absence means, so the call site supplies nothing. One implementation per transport, each
+/// holding that chain's status table.
 pub(crate) trait ClassifyRpcOutcome {
     type Response;
 

crates/foreign-chain-inspector/tests/aptos_inspector.rs

@@ -209,6 +209,29 @@ async fn extract__should_classify_http_errors_by_status(
     assert_eq!(error.is_transient(), expected_transient);
 }
 
+#[tokio::test]
+async fn extract__should_reject_a_response_that_does_not_carry_the_resource() {
+    // Given — a URL that answers but serves something other than the Aptos REST API.
+    let server = MockServer::start();
+    server.mock(|when, then| {
+        when.method(GET).path(tx_path());
+        then.status(200)
+            .header("content-type", "text/html")
+            .body("<html><body>Sign in to continue</body></html>");
+    });
+    let inspector = inspector_for(&server);
+
+    // When
+    let response = inspector
+        .extract(tx_id(), AptosFinality::Committed, vec![])
+        .await;
+
+    // Then — the endpoint is wrong, not slow, so retrying it cannot help.
+    let error = response.expect_err("extract should fail");
+    assert_matches!(error, ForeignChainInspectionError::MalformedRpcResponse(_));
+    assert!(!error.is_transient());
+}
+
 #[tokio::test]
 async fn extract__should_reject_response_with_mismatched_hash() {
     // Given — the backend echoes a different transaction than queried.

crates/foreign-chain-rpc-interfaces/src/aptos.rs

@@ -1,6 +1,6 @@
 use reqwest::Url;
 use reqwest::header::{HeaderMap, HeaderName, HeaderValue};
-use serde::Deserialize;
+use serde::{Deserialize, de::DeserializeOwned};
 use std::future::Future;
 use std::time::Duration;
 
@@ -44,9 +44,11 @@ pub struct EventGuid {
 #[derive(Debug, thiserror::Error)]
 pub enum AptosRpcError {
     #[error("HTTP request failed: {0}")]
-    Http(#[from] reqwest::Error),
+    Http(reqwest::Error),
     #[error("Aptos API returned HTTP {status}: {body}")]
     ApiError { status: u16, body: String },
+    #[error("failed to decode the Aptos API response: {0}")]
+    MalformedBody(serde_json::Error),
 }
 
 /// Partial response of the API root: the ledger info every Aptos node reports.
@@ -106,6 +108,26 @@ impl ReqwestAptosClient {
             .expect("Aptos rpc_url is validated as a URL by node-config before reaching here");
         Self { base, client }
     }
+
+    async fn get_json<T: DeserializeOwned>(&self, url: Url) -> Result<T, AptosRpcError> {
+        let response = self
+            .client
+            .get(url)
+            .send()
+            .await
+            .map_err(AptosRpcError::Http)?;
+        let status = response.status();
+        if !status.is_success() {
+            let body = response.text().await.unwrap_or_default();
+            return Err(AptosRpcError::ApiError {
+                status: status.as_u16(),
+                body,
+            });
+        }
+
+        let body = response.bytes().await.map_err(AptosRpcError::Http)?;
+        serde_json::from_slice(&body).map_err(AptosRpcError::MalformedBody)
+    }
 }
 
 /// Appends `transactions/by_hash/{hash}` to `base`, preserving its path and query string (so a
@@ -124,42 +146,14 @@ impl AptosRpcClient for ReqwestAptosClient {
         &self,
         tx_hash_hex: &str,
     ) -> impl Future<Output = Result<TransactionResponse, AptosRpcError>> + Send {
-        let url = build_request_url(&self.base, tx_hash_hex);
-        let client = self.client.clone();
-        async move {
-            let response = client.get(url).send().await?;
-            let status = response.status();
-            if !status.is_success() {
-                let body = response.text().await.unwrap_or_default();
-                return Err(AptosRpcError::ApiError {
-                    status: status.as_u16(),
-                    body,
-                });
-            }
-            let parsed = response.json::<TransactionResponse>().await?;
-            Ok(parsed)
-        }
+        self.get_json(build_request_url(&self.base, tx_hash_hex))
     }
 
-    /// The ledger info lives at the API root, so the base URL is requested as configured.
     fn get_ledger_info(
         &self,
     ) -> impl Future<Output = Result<LedgerInfoResponse, AptosRpcError>> + Send {
-        let url = self.base.clone();
-        let client = self.client.clone();
-        async move {
-            let response = client.get(url).send().await?;
-            let status = response.status();
-            if !status.is_success() {
-                let body = response.text().await.unwrap_or_default();
-                return Err(AptosRpcError::ApiError {
-                    status: status.as_u16(),
-                    body,
-                });
-            }
-            let parsed = response.json::<LedgerInfoResponse>().await?;
-            Ok(parsed)
-        }
+        // The ledger info lives at the API root.
+        self.get_json(self.base.clone())
     }
 }