fix: fix task nonstopping error if model key is expired

[bytecii]

Description

Validate the model before the task starts and shows the errors if the basic validation fails.

Examples:

Fixes #1010
Fixes #1034

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[LuoPengcheng12138]

some questions:

1. In backend/app/service/error_handler.py, it seems difficult to ensure that ModelProcessingError is only raised when item.action == Action.new_task_state (the task may directly raise Exception, or the agent may also raise ModelProcessingError for other actions).
2. In backend/app/service/error_handler.py, does the should_stop_task function guarantee that for all BYOK models, the returned error codes are exactly "invalid_api_key", "insufficient_quota", and "model_not_found"?
   Perhaps a better approach is to perform a pre-check on the API key before the task starts, rather than handling the thrown errors during runtime.
   cc @fengju0213 @a7m-1st

[bytecii]

@LuoPengcheng12138 Updated with model validation at first.

[LuoPengcheng12138]

@LuoPengcheng12138 Updated with model validation at first.

hi, @bytecraftii Merge is good to go now. One more thing: we need to remove the Python pre-commit hook, as Wendong pointed out that we’re making a large number of updates to the project right now, and adding changes like linting rules will make PR reviews much more difficult.

[bytecii]

@LuoPengcheng12138 Updated with model validation at first.

hi, @bytecraftii Merge is good to go now. One more thing: we need to remove the Python pre-commit hook, as Wendong pointed out that we’re making a large number of updates to the project right now, and adding changes like linting rules will make PR reviews much more difficult.

For now the pre-commit will not be automatically run, need to be trigger manually. Also I do think try to make the code cleaner will help to make the code review life easier in the long run.

[4pmtong]

Hi @bytecii how about reuse the validation logic in #1155 ?

[nitpicker55555]

Can we change to get_running_loop()? Since get_event_loop() is about deprecation

[nitpicker55555]

Should we treat rate limit as invalid signal? I think rate limit is temporal

[nitpicker55555]

Should we also add a timeout in external? like:

try:
      await asyncio.wait_for(
          loop.run_in_executor(None, lambda: agent.step("test")),
          timeout=30.0
      )
  except asyncio.TimeoutError:
      return False, "timed out"

[Wendong-Fan]

thanks @bytecii ! I think this validation may be doing duplicate work. We already validate the model configuration via /model/validate on the settings page before users start tasks, so adding this would introduce extra latency to every task.

the root cause is in question_confirm when an API error occurs, the exception handler does return True instead of propagating the error. This silently swallows the failure and allows the task to continue running.

A much simpler fix would be to change that return True to raise. This lets the error bubble up to the main loop and be sent back to the frontend via SSE, no additional validation step required, WDYT?

cc @nitpicker55555 @4pmtong @LuoPengcheng12138

[Wendong-Fan]

added enhance PR #1170 based on review, feel free to check

[bytecii]

@Wendong-Fan Ok sounds good