Contrib.social

Throw exceptions on errors

Open on Drupal.org →
Created on 16 November 2023, over 1 year ago

Problem/Motivation

The ZoomApi Version 2.x branch used to throw error exceptions when zoom API failed but now no exceptions are thrown. This change was made from this issue https://www.drupal.org/project/zoomapi/issues/3375573 🐛 Log exceptions but don't rethrow them Fixed but the problem with the change other then breaking code that was using a try and catch block is that all that is returned from a response if it fails is NULL. This is fine for post requests as on success a response is returned but on calls to the zoom api that use put or delete success and failure both return NULL so no logic can be run on these calls.

Steps to reproduce

So when canceling a registration to a webinar

$registrant_id = '1111111111111';
$values = [
   'json' => [
     'action' => 'cancel',
     'registrants' => [
       [
         'id' => $registrant_id,
       ],
     ],
   ],
];
$path = 'webinars/22222222222/registrants/status';
$response = $zoom_client->put($path, $values);

If there was an error say the webinar is already closed the $response returned would be NULL and if it was successful canceling the registrants attendance then the $response is also NULL. With an exception I would know if the call failed and could perform additional logic like providing user feedback for the issue.

Proposed resolution

We should add back the onRequestError method to the zoom client. We could also provide a config setting to disable exceptions in this method if it is not needed but I feel providing exceptions should be default as was in the 2.x branch

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

3.0

Component

Code

Created by

🇨🇦Canada erichomanchuk

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @erichomanchuk
  • @erichomanchuk opened merge request.
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇨🇦Canada erichomanchuk

    I added in a config option to disable the exceptions if that is needed but by default exceptions would be thown.

  • Comment over 1 year ago →
    🇺🇦Ukraine yarik.lutsiuk

    Hello,

    i add patch here with current changes so it can be used via composer as current MR can be updated in future.

  • Comment 4 months ago →
    🇺🇸United States joelsteidl

    I feel like throwing exceptions by default could be a breaking change, so what if we just make it so throwing exceptions is optional. Would that still work for your needs?

  • Comment about 1 month ago →
    🇺🇸United States joelsteidl

    Following up here.

    Since the module is currently not throwing exceptions, it would be a breaking change to start throwing them. What if we made the config option to throw exceptions?

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024