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.

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

Jump to bottom

Introduce OpenAIThrowable #429

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

Introduce OpenAIThrowable #429

wants to merge 1 commit into from

Conversation

VincentLanglet
Copy link

What:

  • Bug Fix
  • New Feature

Description:

Hi @gehrisandro

I'm trying to move #287 forward by splitting it in 3 smaller PRs:

  • First PR to introduce OpenAIThrowable
  • Second PR to introduce UnreadableResponse exception
  • Third PR with only Phpdoc update

The purpose of this PR is to provide an easy way to try/catch method of this lib:

  • It avoids catching \Exception which catch everything
  • It avoids catching OpenAI\Exceptions\ErrorException|OpenAI\Exceptions\InvalidArgumentException|... which is complicated and can miss an exception if a new one is introduced later.

Related:

#287
@VincentLanglet
Copy link
Author

Friendly ping @nunomaduro @gehrisandro,

Is something missing for a review ?
Since #287 got interest but no reaction,
I'm trying to moving it forward with smaller PR. Please tell me if I can do it better :)

Thanks
@bytestream
Copy link

I agree that this is a useful and beneficial improvement. I see no reason why this cannot be merged @nunomaduro
@VincentLanglet
Copy link
Author

Anyway to move forward this PR @gehrisandro @nunomaduro ?

Thanks a lot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants
@VincentLanglet @bytestream