Fix: mark scan as failed on fetch result errors and return 206 status

Updated error handling in `get_results` to mark scans as failed if errors
occur during result fetch.

Returning status code `206` (Partial Content) instead of `500` to indicate
partial completion while preserving scan data integrity. This avoids
marking the entire request as a server error, focusing on the partial
availability of results.

Updated OpenAPI spec to reflect `206` response for partial results in case
of errors.

Added `LambdaScannerBuilder` for testing error scenarios in fetch results.

rust/doc/openapi.yml

@@ -346,6 +346,19 @@ paths:
                   description: "Schema of a list of results response"
                 get results 0-3:
                   $ref: "#/components/examples/scan_results"
+        "206":
+          description: "Partial results, most likely an error occurred while fetching the results"
+          content:
+            application/json:
+              schema:
+                type: "array"
+                items:
+                  $ref: "#/components/schemas/Result"
+              examples:
+                schema:
+                  description: "Schema of a list of results response"
+                get results 0-3:
+                  $ref: "#/components/examples/scan_results"
 
         "400":
           description: "Bad range format"

rust/src/openvasd/controller/entry.rs

@@ -429,7 +429,23 @@ where
                     };
 
                     match ctx.scheduler.get_results(&id, begin, end).await {
-                        Ok(results) => Ok(ctx.response.ok_byte_stream(results).await),
+                        Ok(results) => {
+                            match ctx.scheduler.get_scan(&id).await {
+                                Ok((_, status)) => {
+                                    let status_code = match status.status {
+                                        Phase::Failed => hyper::StatusCode::PARTIAL_CONTENT,
+                                        _ => hyper::StatusCode::OK,
+                                    };
+
+                                    Ok(ctx.response.byte_stream(status_code, results).await)
+                                }
+                                // ignore this error as it isn't directly related to the results
+                                Err(e) => {
+                                    tracing::warn!("Error while fetching scan status: {:?}", e);
+                                    Ok(ctx.response.ok_byte_stream(results).await)
+                                }
+                            }
+                        }
                         Err(crate::storage::Error::NotFound) => {
                             Ok(ctx.response.not_found("scans/results", &id))
                         }
@@ -470,6 +486,7 @@ where
 pub mod client {
     use std::sync::Arc;
 
+    use http::StatusCode;
     use http_body_util::{BodyExt, Empty, Full};
     use hyper::{
         body::Bytes, header::HeaderValue, service::HttpService, HeaderMap, Method, Request,
@@ -482,6 +499,7 @@ pub mod client {
     };
     use serde::Deserialize;
 
+    use crate::storage::inmemory;
     use crate::{
         controller::{ClientIdentifier, Context},
         storage::{file::Storage, NVTStorer, UserNASLStorageForKBandVT},
@@ -506,11 +524,7 @@ pub mod client {
             >,
             FSPluginLoader,
         )>,
-        Arc<
-            UserNASLStorageForKBandVT<
-                crate::storage::inmemory::Storage<crate::crypt::ChaCha20Crypt>,
-            >,
-        >,
+        Arc<UserNASLStorageForKBandVT<inmemory::Storage<crate::crypt::ChaCha20Crypt>>>,
     > {
         use crate::file::tests::{example_feeds, nasl_root};
         let storage = crate::storage::inmemory::Storage::default();
@@ -552,6 +566,25 @@ pub mod client {
         Client::authenticated(scanner, storage)
     }
 
+    pub async fn fails_to_fetch_results() -> Client<
+        scannerlib::scanner::fake::LambdaScanner,
+        Arc<UserNASLStorageForKBandVT<inmemory::Storage<crate::crypt::ChaCha20Crypt>>>,
+    > {
+        // TODO: do we need to bother with the storage?
+        use crate::file::tests::example_feeds;
+        let storage = crate::storage::inmemory::Storage::default();
+        let storage = Arc::new(UserNASLStorageForKBandVT::new(storage));
+        storage
+            .synchronize_feeds(example_feeds().await)
+            .await
+            .unwrap();
+
+        let scanner = scannerlib::scanner::fake::LambdaScannerBuilder::new()
+            .with_fetch_results(|_| Err(scanner::Error::Unexpected("no results".to_string())))
+            .build();
+        Client::authenticated(scanner, storage)
+    }
+
     pub async fn file_based_example_feed(
         prefix: &str,
     ) -> Client<
@@ -648,7 +681,7 @@ pub mod client {
             let result = self
                 .request_empty(Method::GET, KnownPaths::ScanStatus(id.to_string()))
                 .await;
-            self.parsed(result).await
+            self.parsed(result, StatusCode::OK).await
         }
 
         pub async fn header(&self) -> TypeResult<HeaderMap<HeaderValue>> {
@@ -662,14 +695,18 @@ pub mod client {
             let result = self
                 .request_empty(Method::GET, KnownPaths::Scans(Some(id.to_string())))
                 .await;
-            self.parsed(result).await
+            self.parsed(result, StatusCode::OK).await
         }
 
-        pub async fn scan_results(&self, id: &str) -> TypeResult<Vec<models::Result>> {
+        pub async fn scan_results(
+            &self,
+            id: &str,
+            status: StatusCode,
+        ) -> TypeResult<Vec<models::Result>> {
             let result = self
                 .request_empty(Method::GET, KnownPaths::ScanResults(id.to_string(), None))
                 .await;
-            self.parsed(result).await
+            self.parsed(result, status).await
         }
         pub async fn scan_delete(&self, id: &str) -> TypeResult<()> {
             let result = self
@@ -705,7 +742,7 @@ pub mod client {
             let result = self
                 .request_empty(Method::GET, KnownPaths::Scans(None))
                 .await;
-            self.parsed(result).await
+            self.parsed(result, StatusCode::OK).await
         }
 
         // TODO: deal with that static stuff that prevents deserializiation based on Bytes
@@ -731,12 +768,12 @@ pub mod client {
             let result = self
                 .request_json(Method::POST, KnownPaths::Scans(None), scan)
                 .await;
-            self.parsed(result).await
+            self.parsed(result, StatusCode::CREATED).await
         }
 
         pub async fn vts(&self) -> TypeResult<Vec<String>> {
             let result = self.request_empty(Method::GET, KnownPaths::Vts(None)).await;
-            self.parsed(result).await
+            self.parsed(result, StatusCode::OK).await
         }
 
         /// Starts a scan and wait until is finished and returns it status and results
@@ -770,14 +807,19 @@ pub mod client {
             }
         }
 
-        pub async fn parsed<'a, T>(&self, result: HttpResult) -> TypeResult<T>
+        pub async fn parsed<'a, T>(
+            &self,
+            result: HttpResult,
+            expected_status: StatusCode,
+        ) -> TypeResult<T>
         where
             T: for<'de> Deserialize<'de>,
         {
             let resp = result?;
-            if resp.status() != 200 && resp.status() != 201 {
+            if resp.status() != expected_status {
                 return Err(scanner::Error::Unexpected(format!(
-                    "Expected 200 for a body response but got {}",
+                    "Expected {} for a body response but got {}",
+                    expected_status,
                     resp.status()
                 )));
             }
@@ -792,6 +834,7 @@ pub mod client {
 
 #[cfg(test)]
 pub(super) mod tests {
+    use http::StatusCode;
     use scannerlib::models::{Scan, VT};
 
     #[tokio::test]
@@ -820,8 +863,25 @@ pub(super) mod tests {
         assert!(vts.len() > 2);
         let (id, status) = client.scan_finish(&scan).await.unwrap();
         assert_eq!(status.status, scannerlib::models::Phase::Succeeded);
-        let results = client.scan_results(&id).await.unwrap();
+        let results = client.scan_results(&id, StatusCode::OK).await.unwrap();
         assert_eq!(3, results.len());
         client.scan_delete(&id).await.unwrap();
     }
+
+    #[tokio::test]
+    #[tracing_test::traced_test]
+    async fn status_of_internal_error_should_be_reflects() {
+        let client = super::client::fails_to_fetch_results().await;
+
+        let mut scan: Scan = Scan::default();
+        scan.target.hosts.push("localhost".to_string());
+        let (id, status) = client.scan_finish(&scan).await.unwrap();
+        assert_eq!(status.status, scannerlib::models::Phase::Failed);
+        let results = client
+            .scan_results(&id, StatusCode::PARTIAL_CONTENT)
+            .await
+            .unwrap();
+        assert_eq!(0, results.len());
+        client.scan_delete(&id).await.unwrap();
+    }
 }

rust/src/openvasd/response.rs

@@ -171,11 +171,11 @@ impl Response {
     }
 
     #[inline]
-    fn ok_json_response(&self, body: BodyKind) -> Result {
+    fn json_response(&self, status: hyper::StatusCode, body: BodyKind) -> Result {
         match self
             .default_response_builder()
             .header("Content-Type", "application/json")
-            .status(hyper::StatusCode::OK)
+            .status(status)
             .body(body)
         {
             Ok(resp) => resp,
@@ -188,8 +188,9 @@ impl Response {
             }
         }
     }
+
     #[inline]
-    pub async fn ok_byte_stream<T>(&self, mut value: T) -> Result
+    pub async fn byte_stream<T>(&self, status: hyper::StatusCode, mut value: T) -> Result
     where
         T: Iterator<Item = Vec<u8>> + Send + 'static,
     {
@@ -227,7 +228,15 @@ impl Response {
             tracing::debug!("end send values");
             drop(tx);
         });
-        self.ok_json_response(BodyKind::BinaryStream(rx))
+        self.json_response(status, BodyKind::BinaryStream(rx))
+    }
+
+    #[inline]
+    pub async fn ok_byte_stream<T>(&self, value: T) -> Result
+    where
+        T: Iterator<Item = Vec<u8>> + Send + 'static,
+    {
+        self.byte_stream(hyper::StatusCode::OK, value).await
     }
 
     #[inline]
@@ -281,7 +290,10 @@ impl Response {
     }
 
     pub fn ok_static(&self, value: &[u8]) -> Result {
-        self.ok_json_response(BodyKind::Binary(value.to_vec().into()))
+        self.json_response(
+            hyper::StatusCode::OK,
+            BodyKind::Binary(value.to_vec().into()),
+        )
     }
 
     pub fn created<T>(&self, value: &T) -> Result

rust/src/openvasd/scheduling.rs

@@ -265,7 +265,12 @@ where
                     };
                 }
                 Err(e) => {
-                    tracing::warn!(%scan_id, %e, "unable to fetch results");
+                    // TODO: set scan to failed and inform entry to return 500 instead of 200
+                    // Also may remove from running
+                    tracing::warn!(%scan_id, %e, "unable to fetch results, setting scan to failed");
+                    let mut status = self.db.get_status(&scan_id).await?;
+                    status.status = Phase::Failed;
+                    self.db.update_status(&scan_id, status).await?;
                 }
             };
         }