Skip to content

Conversation

@lalitb
Copy link
Member

@lalitb lalitb commented Nov 4, 2025

Fixes #471

Changes

  1. User-Agent header creation

    • Fallback to static "(Unknown) RustGenevaClient/0.1" if formatting fails
    • Log error with error!
  2. MD5 hash formatting

    • Silently ignore write! errors (writing to String never fails)
  3. Hex encoding

    • Log error with error! and return zeros on buffer mismatch
  4. UTF-8 conversion (otlp_encoder.rs:573, 587, 610, 668, 682)

    • Log error with error! and return empty string for trace/span IDs
  5. Timestamp conversion (otlp_encoder.rs:787-806)

    • Log error with error! and fallback to Unix epoch

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 November 4, 2025 02:01
@lalitb lalitb changed the title initial commit fix(geneva uploader): Replace unwrap/expect with graceful error handling Nov 4, 2025
@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 51.51515% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.8%. Comparing base (b49f00e) to head (d3bed22).

Files with missing lines Patch % Lines
...eneva-uploader/src/payload_encoder/otlp_encoder.rs 53.8% 12 Missing ⚠️
...eneva/geneva-uploader/src/config_service/client.rs 42.8% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main    #485     +/-   ##
=======================================
- Coverage   53.8%   53.8%   -0.1%     
=======================================
  Files         71      71             
  Lines      11687   11707     +20     
=======================================
+ Hits        6299    6304      +5     
- Misses      5388    5403     +15     

☔ 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.

@lalitb lalitb requested a review from Copilot November 4, 2025 19:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves error handling by replacing .unwrap() calls with more graceful error handling strategies throughout the encoder and config client code. The changes add proper error logging using the tracing::error! macro and provide fallback values to prevent panics in production.

  • Replaced .unwrap() calls with .unwrap_or_else() or if let Err() patterns
  • Added structured error logging with contextual information for debugging
  • Provided sensible fallback values for error cases (empty strings, zeros, epoch timestamps, static headers)

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
otlp_encoder.rs Replaced unwrap calls with error logging and fallbacks for string formatting, UTF-8 conversion, hex encoding, and timestamp conversion
client.rs Added error handling for User-Agent header creation with fallback to static header

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Geneva Exporter] Check usage of unwrap()

2 participants