Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Regression in failure callback handling #8013

Closed
B-Step62 opened this issue Jan 27, 2025 · 2 comments · Fixed by #8032
Closed

[Bug]: Regression in failure callback handling #8013

B-Step62 opened this issue Jan 27, 2025 · 2 comments · Fixed by #8032
Assignees
Labels
bug Something isn't working urgent

Comments

@B-Step62
Copy link
Contributor

B-Step62 commented Jan 27, 2025

What happened?

Problem

In the latest version, failure callbacks added as strings are ignored.

Reproducing code

  1. pip install mlflow
  2. Add callbacks as atrings
import litellm

litellm.success_callback = ["mlflow"]
litellm.failure_callback = ["mlflow"]
  1. Invoke completion in invalid way to cause a failure
litellm.completion(model="invalid", messages=[])
  1. In LiteLLM <= 1.59.3, the callback is invoked and a trace is created. However, in the latest version, it does not happen.
import mlflow

assert mlflow.get_last_active_trace() is not None # <- Only fails in > 1.59.3

(Potential) Root Cause

#7905 changed the handling for custom callbacks when it is added as a string.

import litellm

litellm.success_callback = ["mlflow"]
litellm.failure_callback = ["mlflow"]

Previously, when some function running after this setup, the function_setup utility converts both success and failure callbacks to the actual callback object MlflowLogger.

# Execute some function
litellm.completion(model=...)

print(litellm.success_callback) # Output: [<litellm.integrations.mlflow.MlflowLogger object at 0x11e422700>]
print(litellm.failure_callback) # Output: ['mlflow', <litellm.integrations.mlflow.MlflowLogger at 0x3256ad310>]

However, in the latest version (1.59.7), only success callbacks are converted to the callback objects.

# Execute some function
litellm.completion(model=...)

print(litellm.success_callback) # Output: [<litellm.integrations.mlflow.MlflowLogger object at 0x11e422700>]
print(litellm.failure_callback) # Output: ["mlflow"]

Before this PR, the function_setup function does this conversion for both handlers. The PR moves this logic out to the _add_custom_logger_callback_to_specific_event function, but it is only invoked for success handler (event) now. As a result, the conversion does not happen for failure handlers.

This makes failure handlers not executed. The event handler in litellm_logging.py only trigger callback when it is CustomLogger instance, not a string. Therefore, the failure callbacks stored as a string will be ignored.

Relevant log output

Are you a ML Ops Team?

Yes

What LiteLLM version are you on ?

v1.59.7

Twitter / LinkedIn details

No response

@B-Step62 B-Step62 added the bug Something isn't working label Jan 27, 2025
@B-Step62 B-Step62 changed the title [Bug]: Regression for failure callback handling [Bug]: Regression in failure callback handling Jan 27, 2025
@krrishdholakia
Copy link
Contributor

Previously, when some function running after this setup, the function_setup utility converts both success and failure callbacks to the actual callback object MlflowLogger.

got it

@krrishdholakia
Copy link
Contributor

Oh i see, since we stopped 'auto' adding any customlogger success callback as a failure event as well, it exposed the issue and we didn't have good testing to catch that.

Thank you for your investigation on this @B-Step62

Working on it now

krrishdholakia added a commit that referenced this issue Jan 28, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Fixes #8013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants