Skip to content

Conversation

@julienh-ssv
Copy link
Contributor

No description provided.

@julienh-ssv julienh-ssv self-assigned this Nov 23, 2025
@julienh-ssv julienh-ssv requested review from a team as code owners November 23, 2025 11:52
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 23, 2025

Greptile Overview

Greptile Summary

This PR centralizes error handling for exporter API endpoints by introducing a new toApiError helper function that produces consistent error responses with rich contextual logging.

Key Changes:

  • Added toApiError function in exporter.go that wraps errors with HTTP status codes and logs them with consistent fields (endpoint, method, path, status, error, request)
  • Replaced all direct api.BadRequestError and api.Error calls across 4 handler files with calls to toApiError
  • Removed duplicate manual error logging statements that were inconsistently applied
  • Fixed variable shadowing issue by renaming loop variable from r to roleVal in decided.go

Benefits:

  • All exporter API errors now logged with same structured format, improving observability
  • Consistent error context makes debugging easier (can now search logs by endpoint name)
  • Reduced code duplication by centralizing error handling logic

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are straightforward refactoring that improves logging consistency without altering business logic. The new centralized error handler properly wraps existing error types and adds structured logging. The variable renaming fix (r to roleVal) eliminates shadowing issues. All changes maintain backward compatibility in API responses.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
api/handlers/exporter/exporter.go 5/5 Introduces new centralized toApiError function with consistent error logging across all exporter API handlers
api/handlers/exporter/committee.go 5/5 Updated to use new toApiError function for consistent logging of committee trace errors
api/handlers/exporter/decided.go 5/5 Replaced manual error logging with toApiError calls; renamed loop variable from r to roleVal to avoid shadowing
api/handlers/exporter/validator.go 5/5 Updated to use centralized error handling via toApiError for validator trace endpoints

Sequence Diagram

sequenceDiagram
    participant Client
    participant Handler as API Handler
    participant toApiError
    participant Logger
    participant Response as ErrorResponse

    Client->>Handler: HTTP Request (with invalid params)
    Handler->>Handler: api.Bind() / validate()
    
    alt Validation Error
        Handler->>toApiError: (logger, r, endpoint, 400, request, err)
        toApiError->>Response: Create ErrorResponse (BadRequestError)
        toApiError->>Logger: Error("exporter API request failed", fields...)
        toApiError-->>Handler: ErrorResponse
        Handler-->>Client: HTTP 400 + JSON error
    end

    alt Internal Server Error
        Handler->>Handler: Process request (store.Get...)
        Handler->>toApiError: (logger, r, endpoint, 500, request, err)
        toApiError->>Response: Create ErrorResponse (custom 500)
        toApiError->>Logger: Error("exporter API request failed", fields...)
        toApiError-->>Handler: ErrorResponse
        Handler-->>Client: HTTP 500 + JSON error
    end

    Note over toApiError,Logger: All error logging now centralized<br/>with consistent fields: endpoint,<br/>method, path, status, error, request
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@codecov
Copy link

codecov bot commented Nov 23, 2025

Codecov Report

❌ Patch coverage is 90.24390% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.2%. Comparing base (717cc7b) to head (517f46c).

Files with missing lines Patch % Lines
api/handlers/exporter/exporter.go 90.9% 1 Missing and 1 partial ⚠️
api/handlers/exporter/decided.go 91.6% 1 Missing ⚠️
api/handlers/exporter/validator.go 75.0% 1 Missing ⚠️

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

Copy link
Contributor

@iurii-ssv iurii-ssv left a comment

Choose a reason for hiding this comment

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

Nice!

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.

3 participants