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

KMP-3014: improve error generation #429

Merged
merged 8 commits into from
Jul 26, 2022

Conversation

Tekkon
Copy link

@Tekkon Tekkon commented Jul 8, 2022

It's hard to handle errors generated by the gem in the App which uses it because the errors are too common. There is only Zoom::AuthenticationError and Zoom::Error for any other cases.

This PR improves error generation. The error classes added for common http codes.
https://marketplace.zoom.us/docs/api-reference/error-definitions is used for reference.

beinghaziq and others added 8 commits June 20, 2022 18:08
…g_language_interpretation_for_webinars

KMP-3217 let webinars update API accepts language interpretation in settings
KMP-3232: Server-to-Server OAuth support
Fetch changes from the original zoom_rb repository.
@kyleboe kyleboe self-requested a review July 25, 2022 16:14
Copy link
Owner

@kyleboe kyleboe 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! Thanks again for the contribution. I'll get this merged in and a new gem cut here shortly.

@kyleboe kyleboe merged commit 005c5f2 into kyleboe:main Jul 26, 2022
@Tekkon Tekkon deleted the KMP-3014-improve-error-generation branch July 27, 2022 08:17
@tjefferson08
Copy link

Isn't this a breaking change? If I have rescue Zoom::Error in my codebase, raising these new, granular errors will no longer be caught (since they don't inherit from Zoom::Error)

(do I have that right?)

@kyleboe kyleboe mentioned this pull request Aug 3, 2022
@kyleboe
Copy link
Owner

kyleboe commented Aug 3, 2022

@tjefferson08 you're totally right. Apologies if this caused any disruption. I opened up #435 with a couple smoke tests to ensure we are descending from Zoom::Error. Thanks for catching this.

@tjefferson08
Copy link

Thanks for the quick response & repair! 🥇

@kyleboe
Copy link
Owner

kyleboe commented Aug 10, 2022

@tjefferson08 to close the loop on this, the fix is included in 1.1.4

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.

4 participants