search

zulip / python-zulip-api

Python library for the Zulip API.
https://zulip.com/api/
Apache License 2.0
350 stars 356 forks source link

Raise exceptions for `{"result": "error"}` in `zulip` apis. #672

Open LoopThrough-i-j opened 3 years ago

LoopThrough-i-j commented 3 years ago

Currently, zulip API doesn't handle {"result": "error"} and sends send back whatever comes from the server. We would want to handle these errors by raising exceptions in the API layer whenever there is a {"result": "error"}.

Reference

alexzw7 commented 3 years ago

@zulipbot claim

zulipbot commented 3 years ago

Hello @alexzw7!

Thanks for your interest in Zulip! You have attempted to claim an issue without the labels "help wanted", "good first issue". Since you're a new contributor, you can only claim and submit pull requests for issues with the help wanted or good first issue labels.

If this is your first time here, we recommend reading our guide for new contributors before getting started.

alexzw7 commented 3 years ago

@zulipbot add "good first issue"

alexzw7 commented 3 years ago

@zulipbot claim

zulipbot commented 3 years ago

Welcome to Zulip, @alexzw7! We just sent you an invite to collaborate on this repository at https://github.com/zulip/python-zulip-api/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

LoopThrough-i-j commented 3 years ago

@zulipbot remove "good first issue"

PIG208 commented 3 years ago

A brief note for this one here: we might need to add a common exception class called BotError and specify a set of sub errors like AccessDenied etc. (we need to discuss the naming). I think we can have a protocol that can be shared with embedded bots as well. An anticipated result of this might be that the user will no longer need to examine the returned dictionary from the API and instead they can simply do error handling with try-except statements.

timabbott commented 3 years ago

Yeah, I think that's a good idea.

neiljp commented 3 years ago

Is it the intent of this issue to evolve all of the zulip python API to have different return values and raise exceptions instead? While ZT can adapt to this per zulip library release, this seems like a big API transition to make for all dependent packages/scripts.

If this is an evolution of zulip, then BotError seems misnamed, unless this issue is specific to the bots library instead?

timabbott commented 2 years ago

I think so. One theory is that we could provide a legacy_exception_handling flag to the Client() constructor that resulted in it using the old error-handling method of converting errors for backwards-compatibility.