Skip to content

Conversation

@kxzk
Copy link
Collaborator

@kxzk kxzk commented Feb 8, 2026

TL;DR

Extract 12 identical 5-line Faraday rescue blocks into a single with_faraday_error_handling wrapper method.

Why

Every public method in ApiClient repeated the same RetriableResponse → handle_response, Error → raise ApiError rescue pattern. This was flagged as pre-existing tech debt. The new block-based wrapper eliminates ~60 lines of duplication with zero behavior change.

send_batch and delete_dataset_item retain their own rescue blocks since they use different response handlers (handle_batch_response and handle_delete_dataset_item_response).

Checklist

  • Has label
  • Has linked issue
  • Tests added for new behavior
  • Docs updated (if user-facing)

Closes ML-558

12 public methods repeated the same 5-line rescue block for
RetriableResponse and Faraday::Error. Extract a single
`with_faraday_error_handling` wrapper method.

send_batch and delete_dataset_item retain their own rescue
blocks since they use different response handlers.
@kxzk kxzk added the refactor Code refactoring and tech debt reduction label Feb 8, 2026
Copilot AI review requested due to automatic review settings February 8, 2026 22:47
@linear
Copy link

linear bot commented Feb 8, 2026

@greptile-apps
Copy link

greptile-apps bot commented Feb 8, 2026

Greptile Overview

Greptile Summary

This PR refactors Langfuse::ApiClient to eliminate duplicated Faraday error handling by introducing a single with_faraday_error_handling wrapper and applying it to the API methods that shared an identical rescue pattern. Methods with distinct response handlers (send_batch, delete_dataset_item) intentionally keep custom rescue logic.

Net effect is a readability/maintainability improvement with the same request/response flow: make Faraday call → handle_response (or method-specific handler) → convert Faraday::RetriableResponse into handling of the final response and convert other Faraday::Errors into ApiError with logging.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk.
  • Change is a straightforward refactor that preserves the exact rescue behavior by centralizing identical Faraday exception handling into a wrapper; affected methods still call the same response handlers and the two special-case methods retain custom logic.
  • No files require special attention

Important Files Changed

Filename Overview
lib/langfuse/api_client.rb Refactors repeated Faraday rescue blocks into a private with_faraday_error_handling wrapper and applies it across multiple API methods; behavior appears unchanged.

Sequence Diagram

sequenceDiagram
  participant Caller
  participant ApiClient
  participant Faraday as Faraday::Connection
  participant Retry as Faraday::Retry
  participant Handler as handle_response

  Caller->>ApiClient: list_datasets(...)
  ApiClient->>ApiClient: with_faraday_error_handling { ... }
  ApiClient->>Faraday: get/post/patch/delete
  Faraday->>Retry: apply retry middleware
  alt success response
    Retry-->>Faraday: Faraday::Response
    Faraday-->>ApiClient: response
    ApiClient->>Handler: handle_response(response)
    Handler-->>ApiClient: parsed body (or transformed)
    ApiClient-->>Caller: return value
  else retries exhausted
    Retry-->>Faraday: raise Faraday::RetriableResponse (with response)
    Faraday-->>ApiClient: exception
    ApiClient->>ApiClient: rescue Faraday::RetriableResponse
    ApiClient->>Handler: handle_response(e.response)
    Handler-->>ApiClient: parsed body or raises ApiError
    ApiClient-->>Caller: return/raise
  else other Faraday error
    Retry-->>Faraday: raise Faraday::Error
    Faraday-->>ApiClient: exception
    ApiClient->>ApiClient: rescue Faraday::Error
    ApiClient-->>Caller: raise ApiError
  end

Loading

Copy link

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

Refactors Langfuse::ApiClient to remove repeated Faraday rescue/response-handling code by introducing a shared wrapper for standard request error handling.

Changes:

  • Added a private with_faraday_error_handling helper to centralize Faraday::RetriableResponse / Faraday::Error handling.
  • Updated multiple API methods to execute their request/handle_response logic inside the new wrapper.
  • Kept custom error-handling paths intact for endpoints with different response handlers (e.g., batch and delete).

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

@kxzk kxzk merged commit 620feb2 into main Feb 8, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactoring and tech debt reduction

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant