Skip to content

[WIP] bpo-39725: Suppress only handled exception by "raise ... from ...".#18629

Draft
serhiy-storchaka wants to merge 1 commit intopython:mainfrom
serhiy-storchaka:suppress-context
Draft

[WIP] bpo-39725: Suppress only handled exception by "raise ... from ...".#18629
serhiy-storchaka wants to merge 1 commit intopython:mainfrom
serhiy-storchaka:suppress-context

Conversation

@serhiy-storchaka
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka commented Feb 23, 2020

def _report_invalid_netmask(cls, netmask_str, suppress=True):
msg = '%r is not a valid netmask' % netmask_str
raise NetmaskValueError(msg) from None
if suppress:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases there is no other exception to suppress. When we call this function not from the except block.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check whether sys.exception() is None.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though, do you need to? What's the harm in "from None" when you are not in an except block?

# Check for a netmask or hostmask in dotted-quad form.
# This may raise NetmaskValueError.
prefixlen = cls._prefix_from_ip_string(arg)
prefixlen = cls._prefix_from_string(arg)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was moved to a separate function because it is hard to write it correctly without return. Otherwise we would need to write something like (not tested):

        try:
            prefixlen = cls._prefix_from_prefix_string(arg)
        except NetmaskValueError:
            try:
                prefixlen = cls._prefix_from_ip_string(arg)
            except BaseException as exc:
                raise exc from None

@pganssle
Copy link
Copy Markdown
Member

@serhiy-storchaka Are you sure you got the bpo issue right? Looks unrelated to me.

@serhiy-storchaka
Copy link
Copy Markdown
Member Author

You are right. I copied it from a wrong issue.

@serhiy-storchaka serhiy-storchaka changed the title [WIP] bpo-39019: Suppress only handled exception by "raise ... from ...". [WIP] bpo-39725: Suppress only handled exception by "raise ... from ...". Feb 23, 2020
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 23, 2020

Codecov Report

Merging #18629 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #18629    +/-   ##
========================================
  Coverage   82.06%   82.07%            
========================================
  Files        1955     1955            
  Lines      584068   584254   +186     
  Branches    44458    44460     +2     
========================================
+ Hits       479334   479525   +191     
+ Misses      95110    95103     -7     
- Partials     9624     9626     +2     
Impacted Files Coverage Δ
Lib/idlelib/colorizer.py 85.78% <0.00%> (-1.02%) ⬇️
Lib/_weakrefset.py 59.18% <0.00%> (-0.69%) ⬇️
Modules/_struct.c 89.13% <0.00%> (-0.44%) ⬇️
Lib/test/test_random.py 96.78% <0.00%> (-0.31%) ⬇️
Lib/random.py 88.12% <0.00%> (-0.28%) ⬇️
Modules/clinic/mathmodule.c.h 85.58% <0.00%> (-0.13%) ⬇️
Objects/abstract.c 75.87% <0.00%> (ø) ⬆️
Lib/test/test_math.py 94.84% <0.00%> (+0.01%) ⬆️
Modules/socketmodule.c 75.78% <0.00%> (+0.08%) ⬆️
Lib/test/test_isinstance.py 98.15% <0.00%> (+0.14%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a025d4c...d5aacef. Read the comment docs.

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.

6 participants