koog icon indicating copy to clipboard operation
koog copied to clipboard

Add error handling tests for LLM clients

Open kpavlov opened this issue 6 months ago • 5 comments

  • Add error handling tests for LLM clients (openai, anthropic)

Type of the change

  • [ ] New feature
  • [ ] Bug fix
  • [ ] Documentation fix
  • [x] Tests improvement

Checklist for all pull requests

  • [ ] The pull request has a description of the proposed change
  • [ ] I read the Contributing Guidelines before opening the pull request
  • [ ] The pull request uses develop as the base branch
  • [ ] Tests for the changes have been added
  • [ ] All new and existing tests passed
Additional steps for pull requests adding a new feature
  • [ ] An issue describing the proposed change exists
  • [ ] The pull request includes a link to the issue
  • [ ] The change was discussed and approved in the issue
  • [ ] Docs have been added / updated

kpavlov avatar Jul 13 '25 21:07 kpavlov

What about other clients (GoogleLLMClient, BedrockLLMClient, ...)? Are there any obstacles to cover them as well?

skarpovdev avatar Jul 16 '25 10:07 skarpovdev

What about other clients (GoogleLLMClient, BedrockLLMClient, ...)? Are there any obstacles to cover them as well?

There is Google Gemini mock. Added test for Google LLM

Sometimes, OpenAI mock can be reused, if the api shape is similar. I haven't implemented all possible llm mocks yet due to the limited capacity. There is also possible to write a lower-level SSE response with Mokksy

kpavlov avatar Jul 16 '25 11:07 kpavlov

Side question: Do you think error handling looks right? IllegalStateException seems to be too generic

kpavlov avatar Jul 16 '25 11:07 kpavlov

Do you think error handling looks right? IllegalStateException seems to be too generic

I agree it's quite generic right now and could be improved! 💯

skarpovdev avatar Jul 16 '25 13:07 skarpovdev

Hi @kpavlov , any updates? 🙂

aozherelyeva avatar Jul 28 '25 14:07 aozherelyeva