Skip to content

Conversation

lalitb
Copy link
Member

@lalitb lalitb commented Oct 15, 2025

Summary

Adds optional error message parameters to all FFI functions to provide rich diagnostic
information for debugging and troubleshooting. Error codes alone are insufficient for
diagnosing issues in production - callers need actual error details (e.g., "Invalid UTF-8
in field 'endpoint'" vs just GENEVA_INVALID_CONFIG).

Changes

Rust (src/lib.rs)

  • Added InvalidHandle error code (104) for magic number mismatch
  • Added write_error_if_provided() helper for zero-cost error reporting
  • Updated validate_handle() to return InvalidHandle instead of InvalidData
  • Added (err_msg_out, err_msg_len) parameters to:
    • geneva_client_new()
    • geneva_encode_and_compress_logs()
    • geneva_encode_and_compress_spans()
    • geneva_upload_batch_sync()
  • Changed all Err(_e) to Err(e) to capture actual error context
  • Changed error formatting from Debug ({:?}) to Display for stability

C Headers

  • Updated geneva_errors.h with GENEVA_ERR_INVALID_HANDLE
  • Updated geneva_ffi.h function signatures with error message parameters

Examples

  • Updated logs_example.c and spans_example.c to demonstrate error handling

Usage

Without error details (zero-cost):

if (geneva_client_new(&cfg, &client, NULL, 0) != GENEVA_SUCCESS) {
    log("Failed to create client");
}

With detailed diagnostics:

  char err_buf[512];
  if (geneva_client_new(&cfg, &client, err_buf, sizeof(err_buf)) != GENEVA_SUCCESS) {
      log("Failed to create client: %s", err_buf);  // e.g., "Invalid UTF-8 in field 'tenant'"
  }

Buffer Handling

  • Message shorter than buffer: Full message written, NUL-terminated at actual length
  • Message longer than buffer: Truncated to fit, always NUL-terminated (standard C behavior)
  • Buffer size = 0 or NULL: No error message written, zero allocation overhead
  • All cases: Error code always returned regardless of buffer state

Error messages are always safe and NUL-terminated when buffer provided. Recommended
buffer size: ≥256 bytes for full diagnostics. Thread-safe: no shared state, caller
controls buffer lifetime.

Performance

  • Zero heap allocations when error messages not requested (pass NULL)
  • Zero heap allocations on success path
  • One allocation (immediately freed after copy) when error occurs and caller provides buffer

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@lalitb lalitb requested a review from a team as a code owner October 15, 2025 23:25
@lalitb lalitb changed the title feat Geneva Exporter: Add error message parameters to FFI functions for better diagnostics feat: [Geneva Exporter] Add error message parameters to FFI functions for better diagnostics Oct 15, 2025
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

❌ Patch coverage is 30.00000% with 70 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.0%. Comparing base (a6ce7f5) to head (8db78f3).

Files with missing lines Patch % Lines
...try-exporter-geneva/geneva-uploader-ffi/src/lib.rs 30.0% 70 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main    #474     +/-   ##
=======================================
- Coverage   54.2%   54.0%   -0.2%     
=======================================
  Files         71      71             
  Lines      11436   11501     +65     
=======================================
+ Hits        6203    6220     +17     
- Misses      5233    5281     +48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant