Log exceptions but don't rethrow them

Created on 19 July 2023, over 1 year ago
Updated 16 November 2023, about 1 year ago

Problem/Motivation

apitools logs exceptions with watchdog_exception() in ClientBase.

zoomapi overrides this behaviour to log the exception (not with watchdog_exception() and rethrow the error. Since guzzle throws an exception for 400, 404, 403, 503 etc. responses, this means anything going wrong can break execution. For example I realised this issue because cron was failing to complete.

Steps to reproduce

Proposed resolution

Just don't override the parent onResponseError() method.

Remaining tasks

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

Fixed

Version

3.0

Component

Code

Created by

πŸ‡¬πŸ‡§United Kingdom catch

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

Comments & Activities

  • Issue created by @catch
  • Status changed to Needs review over 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Here's a patch.

  • Status changed to Fixed about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States joelsteidl

    Committed to dev and will be part of 3.0.0-alpha3 soon.

  • Automatically closed - issue fixed for 2 weeks with no activity.

  • Status changed to Fixed about 1 year ago
  • πŸ‡¨πŸ‡¦Canada erichomanchuk

    This change has caused issues with previous code written from the 2.0 API that I was not aware of. The code I had written registers users to zoom and then writes the registration to an event system database on another server. The code I had to do this checked for a exception and did not write to the events system when there was an error with zoom, but with the exception not being throw users who were not successfully registered for zoom got recorded as there was no exception thrown anymore to stop this.

              try {
                $response = $this->zoom->post($zoom_path, $post_values);
              }
              catch (RequestException $exception) {
                // If zoom api fails to remove this registrant then let's not remove
                // the registration from the remote database either.
                $this->messenger->addError($this->t('There seems to be an issue with registration. Please try again later and if the issue persists please contact support at <a href="mailto::email">:email</a>', [':email' => $contact_email]));
                return;
              } 
    

    So now instead of a exception the response value is just NULL. I was not aware of this on the upgrade. So I guess my code needs to change to just testing if the $response is NULL now and I can't get the information from the exception if needed?

  • πŸ‡¨πŸ‡¦Canada erichomanchuk

    Well trying to update my code I ran into an issue with cancelling a registration to a webinar. To do this I have to call $zoom_client->put($zoom_path, $post_values); and then $zoom_client->delete($zoom_path); But a successful call to either returns a NULL value so there is no way to determine what logic I should run next.

    The ClientBase from APiTools only returns $response = $response->getBody()->getContents(); body as a response and for these specific API calls

    /webinars/{webinarId}/registrants/{registrantId}
    /webinars/{webinarId}/registrants/status

    Don't return body contents so the response is always NULL for pass/fail.

Production build 0.71.5 2024