-
Notifications
You must be signed in to change notification settings - Fork 414
[Inference] Improve error handling #1504
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
Conversation
There was a problem hiding this 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 introduces new, more specific error classes for inference requests and replaces the deprecated InferenceOutputError with variants such as HfInferenceProviderOutputError, HfInferenceInputError, and HfInferenceProviderApiError. This change is applied across various provider and utility modules to allow for narrower error handling downstream.
- Replaces references to InferenceOutputError with new error classes.
- Updates error messages for improved contextual information.
- Removes the deprecated InferenceOutputError class.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/inference/src/tasks/audio/automaticSpeechRecognition.ts | Replaced error throw with HfInferenceProviderOutputError. |
packages/inference/src/providers/together.ts | Updated error throws from InferenceOutputError to HfInferenceProviderOutputError. |
packages/inference/src/providers/sambanova.ts | Updated error throws accordingly. |
packages/inference/src/providers/replicate.ts | Updated error messages to use HfInferenceProviderOutputError. |
packages/inference/src/providers/providerHelper.ts | Updated error handling in helper functions. |
packages/inference/src/providers/ovhcloud.ts | Error messages updated to new error classes. |
packages/inference/src/providers/nscale.ts | Error throw updated to HfInferenceProviderOutputError. |
packages/inference/src/providers/novita.ts | Several error throws updated; one now uses HfInferenceInputError. |
packages/inference/src/providers/nebius.ts | Updated error handling to use HfInferenceProviderOutputError. |
packages/inference/src/providers/hyperbolic.ts | Updated error throws to the new error classes. |
packages/inference/src/providers/hf-inference.ts | Many changes replacing InferenceOutputError with new error classes. |
packages/inference/src/providers/featherless-ai.ts | Updated error handling. |
packages/inference/src/providers/fal-ai.ts | Error messages updated for more contextual feedback. |
packages/inference/src/providers/black-forest-labs.ts | Updated error handling; now using HfInference* errors. |
packages/inference/src/lib/makeRequestOptions.ts | Updated thrown errors to new error classes. |
packages/inference/src/lib/getProviderHelper.ts | Updated error messages to use HfInferenceInputError. |
packages/inference/src/lib/getInferenceProviderMapping.ts | Updated error messages to use HfInferenceInputError and HfInferenceHubApiError. |
packages/inference/src/lib/InferenceOutputError.ts | Removed deprecated InferenceOutputError class. |
packages/inference/src/index.ts | Removed export of deprecated error class and updated exports. |
packages/inference/src/error.ts | Introduced new error classes for finer-grained error handling. |
Hey @coyotte508 - would love your input on this PR from a high-level pov |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High-level LGTM
packages/inference/src/index.ts
Outdated
@@ -1,5 +1,5 @@ | |||
export { InferenceClient, InferenceClientEndpoint, HfInference } from "./InferenceClient.js"; | |||
export { InferenceOutputError } from "./lib/InferenceOutputError.js"; | |||
export * from "./error.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export * from "./error.js"; | |
export * from "./errors.js"; |
tiny nit, feel free to completely disregard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my main issue was with the names. HfInference
is a bit confusing and we've tried to move away from that name. InferenceClient
is the name of the main method and is less confusing about the fact that HF does not (generally) run the actual inference. LMKWYT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is green so LGTM
There was a problem hiding this 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 overhauls the inference package’s error handling mechanism by deprecating the old InferenceOutputError and replacing it with a suite of new, more context-rich error types. Key changes include updating error throws across multiple provider files, refactoring helper functions to throw the new error types, and removing the legacy error type entirely.
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/inference/src/providers/*.ts | Replaces InferenceOutputError with appropriate new error types |
packages/inference/src/lib/*.ts | Updates error throwing in helper functions and removes the legacy error |
packages/inference/src/errors.ts | Introduces new error classes and centralizes error detail handling |
packages/inference/README.md | Documents the new error types and their usage |
Comments suppressed due to low confidence (1)
packages/inference/src/providers/fal-ai.ts:297
- Consider abstracting the repeated pattern for handling and formatting API response errors into a shared utility, to reduce duplication and ease future maintenance.
throw new InferenceClientProviderOutputError("Failed to parse result response from fal-ai API: received malformed response");
@@ -33,10 +34,10 @@ export async function makeRequestOptions( | |||
|
|||
// Validate inputs | |||
if (args.endpointUrl && provider !== "hf-inference") { | |||
throw new Error(`Cannot use endpointUrl with a third-party provider.`); | |||
throw new InferenceClientInputError(`Cannot use endpointUrl with a third-party provider.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the new error messages across the codebase follow a consistent style and provide sufficient context; consider centralizing error message formatting to avoid discrepancies in future changes.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice! thank you
let's merge this @SBrandeis and do a release? @kefranabg needs it for another feature too :) |
Introduces new error types to allow narrower error handling downstream
Those errors hold more context, including the underlying http request / response when required
This is a breakin change because we remove the
InferenceOutputError
type and change the error types thrown by the public methods.