python-zulip-api icon indicating copy to clipboard operation
python-zulip-api copied to clipboard

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

Open LoopThrough-i-j opened this issue 3 years ago • 10 comments

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

LoopThrough-i-j avatar Apr 02 '21 08:04 LoopThrough-i-j

@zulipbot claim

alexzw7 avatar Apr 07 '21 02:04 alexzw7

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.

zulipbot avatar Apr 07 '21 02:04 zulipbot

@zulipbot add "good first issue"

alexzw7 avatar Apr 07 '21 02:04 alexzw7

@zulipbot claim

alexzw7 avatar Apr 07 '21 02:04 alexzw7

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)!

zulipbot avatar Apr 07 '21 02:04 zulipbot

@zulipbot remove "good first issue"

LoopThrough-i-j avatar Apr 14 '21 17:04 LoopThrough-i-j

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.

PIG208 avatar May 09 '21 15:05 PIG208

Yeah, I think that's a good idea.

timabbott avatar May 28 '21 16:05 timabbott

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?

neiljp avatar May 28 '21 17:05 neiljp

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.

timabbott avatar Nov 16 '21 03:11 timabbott