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.

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

Chatconv agent: output parser exception #4923

Merged
merged 1 commit into from
May 18, 2023

Conversation

blob42
Copy link
Contributor

@blob42 blob42 commented May 18, 2023

the output parser form chat conversational agent now raises OutputParserException like the rest.

The raise OutputParserExeption(...) from e form also carries through the original error details on what went wrong.

I added the ValueError as a base class to OutputParserException to avoid breaking code that was relying on ValueError as a way to catch exceptions from the agent. So catching ValuError still works. Not sure if this is a good idea though ?

Who can review?

@blob42 blob42 marked this pull request as ready for review May 18, 2023 12:03
@blob42 blob42 changed the title Chatconv output parser exception Chatconv agent: output parser exception May 18, 2023
@blob42 blob42 force-pushed the chatconv_parser_exception branch 2 times, most recently from 56ef889 to 0229e56 Compare May 18, 2023 12:19
Copy link
Contributor

@vowelparrot vowelparrot left a comment

Choose a reason for hiding this comment

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

Looks good to me - what is the reason for changing the inheritance of the exception?

@@ -360,7 +360,7 @@ def dict(self, **kwargs: Any) -> Dict:
return output_parser_dict


class OutputParserException(Exception):
class OutputParserException(ValueError, Exception):
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this needed for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the ValueError as a base class to OutputParserException to avoid breaking code that was relying on ValueError as a way to catch exceptions from the agent. So catching ValuError still works. Not sure if this is a good idea ?

Copy link
Contributor

Choose a reason for hiding this comment

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

good idea imo. but don't need to inherit from Exception in that case (ValueError already subclass of Exception)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true didn't think about it.

@blob42 blob42 force-pushed the chatconv_parser_exception branch from 0229e56 to 29c2de1 Compare May 18, 2023 14:48
@blob42 blob42 force-pushed the chatconv_parser_exception branch from 29c2de1 to 4e42c73 Compare May 18, 2023 21:46
@dev2049
Copy link
Contributor

dev2049 commented May 18, 2023

thanks @blob42 !

@dev2049 dev2049 merged commit 5525b70 into langchain-ai:master May 18, 2023
@danielchalef danielchalef mentioned this pull request Jun 5, 2023
This was referenced Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants