Skip to content

Commit

Permalink
Fix inconsistency between SendResult and other results (#60)
Browse files Browse the repository at this point in the history
Almost all other results:

* `NewProfileExporterV3Result`
* `SerializeResult`
* `PushTagResult`

expose a `...Result_Tag` named `..._RESULT_ERR` and an `err` field
inside their definition, so it's weird to use `failure` just for
`SendResult`, which otherwise looks exactly like all the others.

Here's how `ffi.h` looks after this change:

```diff
 typedef enum ddprof_ffi_SendResult_Tag {
   DDPROF_FFI_SEND_RESULT_HTTP_RESPONSE,
-  DDPROF_FFI_SEND_RESULT_FAILURE,
+  DDPROF_FFI_SEND_RESULT_ERR,
 } ddprof_ffi_SendResult_Tag;

 typedef struct ddprof_ffi_SendResult {
@@ -150,7 +150,7 @@
       struct ddprof_ffi_HttpStatus http_response;
     };
     struct {
-      struct ddprof_ffi_Vec_u8 failure;
+      struct ddprof_ffi_Vec_u8 err;
     };
   };
 } ddprof_ffi_SendResult;
```

The `ParseTagsResult` is still inconsistent (uses `error_message`)
but that one is inconsistent in other ways (doesn't expose a tag,
uses a pointer to a Vec_u8), so I think it's not a bad idea that
`error_message` is different as it needs to be handled differently.
  • Loading branch information
ivoanjo authored and r1viollet committed May 5, 2022
1 parent a82bd9c commit 0711e0d
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
8 changes: 4 additions & 4 deletions ddprof-ffi/src/exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::str::FromStr;
#[repr(C)]
pub enum SendResult {
HttpResponse(HttpStatus),
Failure(crate::Vec<u8>),
Err(crate::Vec<u8>),
}

#[repr(C)]
Expand Down Expand Up @@ -202,15 +202,15 @@ pub unsafe extern "C" fn profile_exporter_send(
let exp_ptr = match exporter {
None => {
let buf: &[u8] = b"Failed to export: exporter was null";
return SendResult::Failure(crate::Vec::from(Vec::from(buf)));
return SendResult::Err(crate::Vec::from(Vec::from(buf)));
}
Some(e) => e,
};

let request_ptr = match request {
None => {
let buf: &[u8] = b"Failed to export: request was null";
return SendResult::Failure(crate::Vec::from(Vec::from(buf)));
return SendResult::Err(crate::Vec::from(Vec::from(buf)));
}
Some(req) => req,
};
Expand All @@ -223,7 +223,7 @@ pub unsafe extern "C" fn profile_exporter_send(
Ok(HttpStatus(response.status().as_u16()))
}() {
Ok(code) => SendResult::HttpResponse(code),
Err(err) => SendResult::Failure(err.into()),
Err(err) => SendResult::Err(err.into()),
}
}

Expand Down
4 changes: 2 additions & 2 deletions examples/ffi/exporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ int main(int argc, char *argv[]) {
int exit_code = 0;
ddprof_ffi_SendResult send_result =
ddprof_ffi_ProfileExporterV3_send(exporter, request, cancel);
if (send_result.tag == DDPROF_FFI_SEND_RESULT_FAILURE) {
print_error("Failed to send profile: ", send_result.failure);
if (send_result.tag == DDPROF_FFI_SEND_RESULT_ERR) {
print_error("Failed to send profile: ", send_result.err);
exit_code = 1;
} else {
printf("Response code: %d\n", send_result.http_response.code);
Expand Down

0 comments on commit 0711e0d

Please sign in to comment.