Skip to content

fix: use configurable max_tokens in credential validation instead of hardcoded value #260

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.

Sign up for GitHub

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

Open
huayaoyue6 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: use configurable max_tokens in credential validation instead of hardcoded value #260

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion python/dify_plugin/interfaces/model/openai_compatible/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ def validate_credentials(self, model: str, credentials: dict) -> None:
endpoint_url += "/"

# prepare the payload for a simple ping to the model
data = {"model": credentials.get("endpoint_model_name", model), "max_tokens": 5}
data = {"model": credentials.get("endpoint_model_name", model),
"max_tokens": int(credentials.get("max_tokens_to_sample", 20))}
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 7, 2025

Choose a reason for hiding this comment

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

critical

This direct int() conversion is unsafe. If credentials.get('max_tokens_to_sample') returns a non-numeric string (e.g., from user configuration), this will raise a ValueError. This exception is caught by the generic except Exception block below, but the error message formatting at line 254 (f"... response body {response.text}") will then raise a NameError because the response variable has not been defined yet. This NameError is unhandled and will likely crash the request handler, providing a confusing stack trace to the user.

To fix this, you should validate the value and handle conversion errors gracefully before it's used. For example, you could use a try-except block around the conversion and raise a CredentialsValidateFailedError with a clear message if it fails.

Comment on lines +186 to +187
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 7, 2025

Choose a reason for hiding this comment

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

high

While this change makes max_tokens configurable for the default validation path, the value is unconditionally overridden with a hardcoded 10 on line 206 when stream validation is enabled (stream_mode_auth == 'use'). This means that for models requiring more than 10 tokens, stream validation will still fail, making this fix incomplete. To fully address the issue described in the pull request, the max_tokens for stream validation should also be configurable or use the value you've set here.


completion_type = LLMMode.value_of(credentials["mode"])

Expand Down