feat!: Align tracer FFI error and response types with common conventions

libdd-data-pipeline-ffi/src/response.rs

@@ -3,8 +3,8 @@
 
 //! Define FFI compatible AgentResponse struct
 
+use libdd_common_ffi::slice::ByteSlice;
 use libdd_data_pipeline::trace_exporter::agent_response::AgentResponse;
-use std::ptr::null;
 
 /// Structure containing the agent response to a trace payload
 /// MUST be freed with `ddog_trace_exporter_response_free`
@@ -29,27 +29,16 @@ impl From<AgentResponse> for ExporterResponse {
     }
 }
 
-/// Return a read-only pointer to the response body. This pointer is only valid as long as
-/// `response` is valid.
+/// Return a borrowed view of the response body.  The returned slice is
+/// only valid as long as `response` is alive.  Returns an empty slice
+/// when `response` is null or the body is absent.
 #[no_mangle]
-pub unsafe extern "C" fn ddog_trace_exporter_response_get_body(
-    response: *const ExporterResponse,
-    out_len: Option<&mut usize>,
-) -> *const u8 {
-    let mut len: usize = 0;
-    let body = if response.is_null() {
-        null()
-    } else if let Some(body) = &(*response).body {
-        len = body.len();
-        body.as_ptr()
-    } else {
-        null()
-    };
-
-    if let Some(out_len) = out_len {
-        *out_len = len;
-    }
-    body
+pub unsafe extern "C" fn ddog_trace_exporter_response_get_body<'a>(
+    response: Option<&'a ExporterResponse>,
+) -> ByteSlice<'a> {
+    response
+        .and_then(|r| Some(ByteSlice::from(r.body.as_deref()?)))
+        .unwrap_or_default()
 }
 
 /// Free `response` and all its contents. After being called response will not point to a valid
@@ -66,40 +55,27 @@ mod tests {
     use super::*;
 
     #[test]
-    fn constructor_test_changed() {
+    fn body_from_changed_response() {
         let agent_response = AgentResponse::Changed {
             body: "res".to_string(),
         };
-        let response = &ExporterResponse::from(agent_response) as *const ExporterResponse;
-        let mut len = 0;
-        let body = unsafe { ddog_trace_exporter_response_get_body(response, Some(&mut len)) };
-        let response =
-            unsafe { std::str::from_utf8(std::slice::from_raw_parts(body, len)).unwrap() };
-        assert_eq!(response, "res");
-        assert_eq!(len, 3);
+        let response = ExporterResponse::from(agent_response);
+        let body = unsafe { ddog_trace_exporter_response_get_body(Some(&response)) };
+        assert_eq!(body.len(), 3);
+        assert_eq!(std::str::from_utf8(&body).unwrap(), "res");
     }
 
     #[test]
-    fn constructor_test_unchanged() {
+    fn body_from_unchanged_response() {
         let agent_response = AgentResponse::Unchanged;
-        let response = Box::into_raw(Box::new(ExporterResponse::from(agent_response)));
-        let mut len = usize::MAX;
-        let body = unsafe { ddog_trace_exporter_response_get_body(response, Some(&mut len)) };
-        assert!(body.is_null());
-        assert_eq!(len, 0);
-
-        unsafe {
-            ddog_trace_exporter_response_free(response);
-        }
+        let response = ExporterResponse::from(agent_response);
+        let body = unsafe { ddog_trace_exporter_response_get_body(Some(&response)) };
+        assert_eq!(body.len(), 0);
     }
 
     #[test]
-    fn handle_null_test() {
-        unsafe {
-            let body = ddog_trace_exporter_response_get_body(null(), None);
-            assert!(body.is_null());
-
-            ddog_trace_exporter_response_free(null::<ExporterResponse>() as *mut ExporterResponse);
-        }
+    fn body_from_null_response() {
+        let body = unsafe { ddog_trace_exporter_response_get_body(None) };
+        assert_eq!(body.len(), 0);
     }
 }

libdd-data-pipeline-ffi/src/tracer.rs

@@ -77,10 +77,10 @@ pub struct TracerSpanFields<'a> {
 #[no_mangle]
 pub unsafe extern "C" fn ddog_tracer_span_new(
     out_handle: NonNull<Box<TracerSpan>>,
-    fields: &TracerSpanFields,
+    fields: Option<&TracerSpanFields>,
 ) -> Option<Box<ExporterError>> {
     catch_panic!(
-        {
+        if let Some(fields) = fields {
             let inner = || -> Result<(), Box<ExporterError>> {
                 let service = charslice_to_bytesstring(fields.service)?;
                 let name = charslice_to_bytesstring(fields.name)?;
@@ -108,6 +108,8 @@ pub unsafe extern "C" fn ddog_tracer_span_new(
                 Ok(())
             };
             inner().err()
+        } else {
+            gen_error!(ErrorCode::InvalidArgument)
         },
         gen_error!(ErrorCode::Panic)
     )
@@ -273,18 +275,19 @@ pub unsafe extern "C" fn ddog_tracer_trace_chunks_begin_chunk(
 #[no_mangle]
 pub unsafe extern "C" fn ddog_tracer_trace_chunks_push_span(
     handle: Option<&mut TracerTraceChunks>,
-    span: Box<TracerSpan>,
+    span: Option<Box<TracerSpan>>,
 ) -> Option<Box<ExporterError>> {
     catch_panic!(
-        if let Some(chunks) = handle {
-            if let Some(chunk) = chunks.0.last_mut() {
-                chunk.push(span.0);
-                None
-            } else {
-                gen_error!(ErrorCode::InvalidArgument)
+        match (handle, span) {
+            (Some(chunks), Some(span)) => {
+                if let Some(chunk) = chunks.0.last_mut() {
+                    chunk.push(span.0);
+                    None
+                } else {
+                    gen_error!(ErrorCode::InvalidArgument)
+                }
             }
-        } else {
-            gen_error!(ErrorCode::InvalidArgument)
+            _ => gen_error!(ErrorCode::InvalidArgument),
         },
         gen_error!(ErrorCode::Panic)
     )
@@ -313,12 +316,15 @@ pub unsafe extern "C" fn ddog_tracer_trace_chunks_push_span(
 #[no_mangle]
 pub unsafe extern "C" fn ddog_trace_exporter_send_trace_chunks(
     exporter: Option<&TraceExporter>,
-    chunks: Box<TracerTraceChunks>,
+    chunks: Option<Box<TracerTraceChunks>>,
     response_out: Option<NonNull<Box<ExporterResponse>>>,
 ) -> Option<Box<ExporterError>> {
     let Some(exporter) = exporter else {
         return gen_error!(ErrorCode::InvalidArgument);
     };
+    let Some(chunks) = chunks else {
+        return gen_error!(ErrorCode::InvalidArgument);
+    };
 
     catch_panic!(
         match exporter.send_trace_chunks(chunks.0) {
@@ -361,7 +367,7 @@ mod tests {
                 duration: 0,
                 error: 0,
             };
-            let err = ddog_tracer_span_new(out, &fields);
+            let err = ddog_tracer_span_new(out, Some(&fields));
             assert!(err.is_none());
             handle.assume_init()
         }
@@ -386,7 +392,7 @@ mod tests {
                 duration: 25_000_000,
                 error: 0,
             };
-            let err = ddog_tracer_span_new(out, &fields);
+            let err = ddog_tracer_span_new(out, Some(&fields));
             assert!(err.is_none());
 
             let span = handle.assume_init();
@@ -500,7 +506,7 @@ mod tests {
                 duration: 0,
                 error: 0,
             };
-            let err = ddog_tracer_span_new(out, &fields);
+            let err = ddog_tracer_span_new(out, Some(&fields));
             assert!(err.is_none());
 
             let span = handle.assume_init();
@@ -533,18 +539,18 @@ mod tests {
             assert!(err.is_none());
 
             let s1 = make_minimal_span();
-            let err = ddog_tracer_trace_chunks_push_span(Some(&mut *chunks), s1);
+            let err = ddog_tracer_trace_chunks_push_span(Some(&mut *chunks), Some(s1));
             assert!(err.is_none());
 
             let s2 = make_minimal_span();
-            let err = ddog_tracer_trace_chunks_push_span(Some(&mut *chunks), s2);
+            let err = ddog_tracer_trace_chunks_push_span(Some(&mut *chunks), Some(s2));
             assert!(err.is_none());
 
             // Chunk 2: one span
             let err = ddog_tracer_trace_chunks_begin_chunk(Some(&mut *chunks), 1);
             assert!(err.is_none());
             let s3 = make_minimal_span();
-            let err = ddog_tracer_trace_chunks_push_span(Some(&mut *chunks), s3);
+            let err = ddog_tracer_trace_chunks_push_span(Some(&mut *chunks), Some(s3));
             assert!(err.is_none());
 
             assert_eq!(chunks.0.len(), 2);
@@ -571,14 +577,39 @@ mod tests {
 
             // No begin_chunk — push should fail
             let s = make_minimal_span();
-            let err = ddog_tracer_trace_chunks_push_span(Some(&mut *chunks), s);
+            let err = ddog_tracer_trace_chunks_push_span(Some(&mut *chunks), Some(s));
             assert!(err.is_some());
             ddog_trace_exporter_error_free(err);
 
             ddog_tracer_trace_chunks_free(chunks);
         }
     }
 
+    #[test]
+    fn push_span_null_span_returns_error() {
+        unsafe {
+            let mut chunks = make_chunks(1);
+            let err = ddog_tracer_trace_chunks_begin_chunk(Some(&mut *chunks), 0);
+            assert!(err.is_none());
+
+            let err = ddog_tracer_trace_chunks_push_span(Some(&mut *chunks), None);
+            assert!(err.is_some());
+            ddog_trace_exporter_error_free(err);
+
+            ddog_tracer_trace_chunks_free(chunks);
+        }
+    }
+
+    #[test]
+    fn push_span_null_handle_returns_error() {
+        unsafe {
+            let s = make_minimal_span();
+            let err = ddog_tracer_trace_chunks_push_span(None, Some(s));
+            assert!(err.is_some());
+            ddog_trace_exporter_error_free(err);
+        }
+    }
+
     #[test]
     fn trace_chunks_empty_is_valid() {
         unsafe {
@@ -601,4 +632,57 @@ mod tests {
             ddog_tracer_trace_chunks_free(chunks);
         }
     }
+
+    #[test]
+    fn span_new_null_fields_returns_error() {
+        unsafe {
+            let mut handle = MaybeUninit::<Box<TracerSpan>>::uninit();
+            let out = NonNull::new(handle.as_mut_ptr()).unwrap();
+            let err = ddog_tracer_span_new(out, None);
+            assert!(err.is_some());
+            assert_eq!(err.as_ref().unwrap().code, ErrorCode::InvalidArgument);
+            ddog_trace_exporter_error_free(err);
+        }
+    }
+
+    #[test]
+    fn send_trace_chunks_null_exporter_returns_error() {
+        unsafe {
+            let chunks = make_chunks(0);
+            let err = ddog_trace_exporter_send_trace_chunks(None, Some(chunks), None);
+            assert!(err.is_some());
+            assert_eq!(err.as_ref().unwrap().code, ErrorCode::InvalidArgument);
+            ddog_trace_exporter_error_free(err);
+        }
+    }
+
+    // Capacity-overflow tests: a C caller passing `usize::MAX` would make
+    // `Vec::with_capacity` panic with "capacity overflow"; the `catch_panic!`
+    // guard must convert that into `ErrorCode::Panic` instead of aborting
+    // the process.
+    #[cfg(all(feature = "catch_panic", panic = "unwind"))]
+    #[test]
+    fn trace_chunks_new_with_overflow_capacity_returns_panic_error() {
+        unsafe {
+            let mut handle = MaybeUninit::<Box<TracerTraceChunks>>::uninit();
+            let out = NonNull::new(handle.as_mut_ptr()).unwrap();
+            let err = ddog_tracer_trace_chunks_new(usize::MAX, out);
+            assert!(err.is_some());
+            assert_eq!(err.as_ref().unwrap().code, ErrorCode::Panic);
+            ddog_trace_exporter_error_free(err);
+        }
+    }
+
+    #[cfg(all(feature = "catch_panic", panic = "unwind"))]
+    #[test]
+    fn begin_chunk_with_overflow_capacity_returns_panic_error() {
+        unsafe {
+            let mut chunks = make_chunks(0);
+            let err = ddog_tracer_trace_chunks_begin_chunk(Some(&mut *chunks), usize::MAX);
+            assert!(err.is_some());
+            assert_eq!(err.as_ref().unwrap().code, ErrorCode::Panic);
+            ddog_trace_exporter_error_free(err);
+            ddog_tracer_trace_chunks_free(chunks);
+        }
+    }
 }