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

Log exceptions and fix erroneous handling #46

Merged
merged 6 commits into from
Nov 20, 2024

Conversation

DavidHuber-NOAA
Copy link
Collaborator

@DavidHuber-NOAA DavidHuber-NOAA commented Nov 15, 2024

Description

This fixes a few custom exceptions. Attempting to raise an exception object results in an unintended/misleading error. For instance,

try:
    1/0
except Exception as exc:
    raise exc("You can't divide by zero")

Results in

Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
TypeError: 'ZeroDivisionError' object is not callable

Instead, the base exception should be casted as a string:

try:
   1/0
except Exception as e:
   raise Exception(str(e) + "\nYou can't divide by zero")

Which returns

Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
Exception: division by zero
You can't divide by zero

In many cases, this is purely redundant as the message will already appear above. Thus, some exceptions were removed entirely or condensed.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • pynorms
  • pytests

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 64.28571% with 5 lines in your changes missing coverage. Please review.

Project coverage is 52.79%. Comparing base (a7b49e9) to head (abe7fd4).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
src/wxflow/fsutils.py 25.00% 3 Missing ⚠️
src/wxflow/schema.py 0.00% 1 Missing ⚠️
src/wxflow/sqlitedb.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop      #46      +/-   ##
===========================================
+ Coverage    52.41%   52.79%   +0.38%     
===========================================
  Files           18       18              
  Lines         1679     1680       +1     
  Branches       296      296              
===========================================
+ Hits           880      887       +7     
+ Misses         749      743       -6     
  Partials        50       50              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@WalterKolczynski-NOAA
Copy link
Contributor

WalterKolczynski-NOAA commented Nov 19, 2024

Why not just reraise the exception?

try:
   1/0
except Exception as exc:
   logger.exception("You can't divide by zero")
   raise exc

I dislike changing the exception type and inserting a possibly different one. The only reason we should be catching these in the first place is to write to the logger. Then just raise the same exception again if we aren't handling it.

@DavidHuber-NOAA
Copy link
Collaborator Author

Alright, I can work with that.

@DavidHuber-NOAA
Copy link
Collaborator Author

@WalterKolczynski-NOAA Alright, I added some logging and re-raising of caught exceptions.

@DavidHuber-NOAA DavidHuber-NOAA changed the title Fix/exceptions Log exceptions and fix erroneous handling Nov 19, 2024
@DavidHuber-NOAA DavidHuber-NOAA merged commit 3ea5acd into NOAA-EMC:develop Nov 20, 2024
9 checks passed
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.

2 participants