Refactor error message handling in FFI functions to return allocated C strings instead of using provided buffers. Updated related tests to reflect changes in error handling functions.

Author: jimkring

src/ffi/request.rs

@@ -251,11 +251,10 @@ pub extern "C" fn request_read_error_kind(request_id: RequestId) -> u8 {
 #[unsafe(no_mangle)]
 pub extern "C" fn request_read_error_message(
     request_id: RequestId,
-    buffer: *mut c_char,
-    buffer_len: usize,
-) -> usize {
-    if buffer.is_null() || buffer_len == 0 {
-        return 0;
+    num_bytes: *mut u32,
+) -> *mut c_char {
+    if num_bytes.is_null() {
+        return ptr::null_mut();
     }
 
     let tracker = REQUEST_TRACKER.lock().unwrap();
@@ -264,28 +263,43 @@ pub extern "C" fn request_read_error_message(
         let progress = progress_info.read().unwrap();
         if let Some(ref response) = progress.final_response {
             if let Err(ref error_msg) = response.body {
-                let msg_len = error_msg.len().min(buffer_len - 1);
+                let msg_len = error_msg.len();
+                unsafe { *num_bytes = msg_len as u32 };
+
+                // Allocate memory for the string + null terminator
+                let c_str_ptr = unsafe { libc::malloc(msg_len + 1) as *mut c_char };
+                if c_str_ptr.is_null() {
+                    unsafe { *num_bytes = 0 };
+                    return ptr::null_mut();
+                }
+
+                // Copy the string and add null terminator
                 unsafe {
-                    std::ptr::copy_nonoverlapping(error_msg.as_ptr(), buffer as *mut u8, msg_len);
-                    *buffer.add(msg_len) = 0;
+                    std::ptr::copy_nonoverlapping(
+                        error_msg.as_ptr(),
+                        c_str_ptr as *mut u8,
+                        msg_len,
+                    );
+                    *(c_str_ptr.add(msg_len)) = 0;
                 }
-                return msg_len;
+
+                return c_str_ptr;
             }
         }
     }
 
-    0
+    unsafe { *num_bytes = 0 };
+    ptr::null_mut()
 }
 
 /// Get the error URL if available
 #[unsafe(no_mangle)]
 pub extern "C" fn request_read_error_url(
     request_id: RequestId,
-    buffer: *mut c_char,
-    buffer_len: usize,
-) -> usize {
-    if buffer.is_null() || buffer_len == 0 {
-        return 0;
+    num_bytes: *mut u32,
+) -> *mut c_char {
+    if num_bytes.is_null() {
+        return ptr::null_mut();
     }
 
     let tracker = REQUEST_TRACKER.lock().unwrap();
@@ -294,28 +308,39 @@ pub extern "C" fn request_read_error_url(
         let progress = progress_info.read().unwrap();
         if let Some(ref response) = progress.final_response {
             if let Some(ref url) = response.error_url {
-                let url_len = url.len().min(buffer_len - 1);
+                let url_len = url.len();
+                unsafe { *num_bytes = url_len as u32 };
+
+                // Allocate memory for the string + null terminator
+                let c_str_ptr = unsafe { libc::malloc(url_len + 1) as *mut c_char };
+                if c_str_ptr.is_null() {
+                    unsafe { *num_bytes = 0 };
+                    return ptr::null_mut();
+                }
+
+                // Copy the string and add null terminator
                 unsafe {
-                    std::ptr::copy_nonoverlapping(url.as_ptr(), buffer as *mut u8, url_len);
-                    *buffer.add(url_len) = 0;
+                    std::ptr::copy_nonoverlapping(url.as_ptr(), c_str_ptr as *mut u8, url_len);
+                    *(c_str_ptr.add(url_len)) = 0;
                 }
-                return url_len;
+
+                return c_str_ptr;
             }
         }
     }
 
-    0
+    unsafe { *num_bytes = 0 };
+    ptr::null_mut()
 }
 
 /// Get the root cause error message
 #[unsafe(no_mangle)]
 pub extern "C" fn request_read_error_source(
     request_id: RequestId,
-    buffer: *mut c_char,
-    buffer_len: usize,
-) -> usize {
-    if buffer.is_null() || buffer_len == 0 {
-        return 0;
+    num_bytes: *mut u32,
+) -> *mut c_char {
+    if num_bytes.is_null() {
+        return ptr::null_mut();
     }
 
     let tracker = REQUEST_TRACKER.lock().unwrap();
@@ -324,17 +349,29 @@ pub extern "C" fn request_read_error_source(
         let progress = progress_info.read().unwrap();
         if let Some(ref response) = progress.final_response {
             if let Some(ref source) = response.error_source {
-                let src_len = source.len().min(buffer_len - 1);
+                let src_len = source.len();
+                unsafe { *num_bytes = src_len as u32 };
+
+                // Allocate memory for the string + null terminator
+                let c_str_ptr = unsafe { libc::malloc(src_len + 1) as *mut c_char };
+                if c_str_ptr.is_null() {
+                    unsafe { *num_bytes = 0 };
+                    return ptr::null_mut();
+                }
+
+                // Copy the string and add null terminator
                 unsafe {
-                    std::ptr::copy_nonoverlapping(source.as_ptr(), buffer as *mut u8, src_len);
-                    *buffer.add(src_len) = 0;
+                    std::ptr::copy_nonoverlapping(source.as_ptr(), c_str_ptr as *mut u8, src_len);
+                    *(c_str_ptr.add(src_len)) = 0;
                 }
-                return src_len;
+
+                return c_str_ptr;
             }
         }
     }
 
-    0
+    unsafe { *num_bytes = 0 };
+    ptr::null_mut()
 }
 
 /// Check if response has an error

tests/integration/common.rs

@@ -41,7 +41,7 @@ pub fn wait_for_request_with_retry(request_id: usize, max_retries: u64) -> (u16,
 
             // Get error message if available for debugging
             let mut error_len: u32 = 0;
-            let error_ptr = request_read_transport_error(request_id, &mut error_len);
+            let error_ptr = request_read_error_message(request_id, &mut error_len);
             let error_msg = if !error_ptr.is_null() && error_len > 0 {
                 let error_str = unsafe {
                     let error_slice =

tests/integration/error_handling.rs

@@ -25,12 +25,12 @@ fn test_error_handling_invalid_url() {
         std::thread::sleep(Duration::from_millis(50));
     }
 
-    // Check if we have a transport error
-    assert!(request_has_transport_error(request_id));
+    // Check if we have an error
+    assert!(request_has_error(request_id));
 
     // Read the error message
     let mut error_len: u32 = 0;
-    let error_ptr = request_read_transport_error(request_id, &mut error_len);
+    let error_ptr = request_read_error_message(request_id, &mut error_len);
     assert!(!error_ptr.is_null());
     assert!(error_len > 0);
 
@@ -81,12 +81,12 @@ fn test_error_handling_connection_timeout() {
         std::thread::sleep(Duration::from_millis(50));
     }
 
-    // Check if we have a transport error
-    assert!(request_has_transport_error(request_id));
+    // Check if we have an error
+    assert!(request_has_error(request_id));
 
     // Read the error message
     let mut error_len: u32 = 0;
-    let error_ptr = request_read_transport_error(request_id, &mut error_len);
+    let error_ptr = request_read_error_message(request_id, &mut error_len);
     assert!(!error_ptr.is_null());
     assert!(error_len > 0);
 
@@ -129,8 +129,8 @@ fn test_error_handling_404_not_found() {
         std::thread::sleep(Duration::from_millis(50));
     }
 
-    // For HTTP errors like 404, we don't expect a transport error
-    assert!(!request_has_transport_error(request_id));
+    // For HTTP errors like 404, we don't expect an error in the body
+    assert!(!request_has_error(request_id));
 
     // But we do expect a 404 status code
     let status = request_read_response_status(request_id);