Fix bad exceptions handlings by 4383 · Pull Request #883 · eventlet/eventlet

4383 · 2024-01-16T10:38:02Z

This patch fix 2 conditions that are badly designed. They can't work. In all case the exception will
be catched by the first except and then the second one will be all the time ignored.

Here is a tiny PoC to demonstrate the bad design:

import errno

def trigger(err, message):
    print(f"try to simulate {message}: {err}")
    raise OSError(err, message)


def catcher(err, message):
    try:
        trigger(err, message)
    except OSError as e:
        print("I'm here")
        if e.errno != errno.EAGAIN:
            raise
    except OSError as e:
        print("I'm there")
        if e.errno == errno.EPIPE:
            return ''
        raise


if __name__ == "__main__":
    try:
        catcher(errno.EPIPE, "pipe!")
    except Exception as e:
        print(e)
    try:
        catcher(errno.EAGAIN, "again!")
    except Exception as e:
        print(e)

The previous code will output:

try to simulate pipe!: 32                                                      
I'm here                                                                       
[Errno 32] pipe!                                                                                                                                               
try to simulate again!: 11     
I'm here

This patch fix 2 conditions that are badly designed. They can't work. In all case the exception will be catched by the first except and then the second one will be all the time ignored.

codecov · 2024-01-16T10:39:53Z

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (4cf3190) 55% compared to head (eed4bb0) 55%.

Files	Patch %	Lines
eventlet/green/os.py	0%	3 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##           master   #883   +/-   ##
=====================================
  Coverage      55%    55%           
=====================================
  Files          89     89           
  Lines        9837   9832    -5     
  Branches     1828   1825    -3     
=====================================
  Hits         5414   5414           
+ Misses       4054   4049    -5     
  Partials      369    369

Flag	Coverage Δ
ipv6	`22% <0%> (+<1%)`	⬆️
py310asyncio	`51% <0%> (+<1%)`	⬆️
py310epolls	`52% <0%> (+<1%)`	⬆️
py310poll	`52% <0%> (+<1%)`	⬆️
py310selects	`52% <0%> (+<1%)`	⬆️
py311asyncio	`51% <0%> (+<1%)`	⬆️
py311epolls	`52% <0%> (+<1%)`	⬆️
py312asyncio	`49% <0%> (-1%)`	⬇️
py312epolls	`50% <0%> (+<1%)`	⬆️
py37epolls	`50% <0%> (+<1%)`	⬆️
py38epolls	`52% <0%> (+<1%)`	⬆️
py38openssl	`51% <0%> (+<1%)`	⬆️
py38poll	`52% <0%> (+<1%)`	⬆️
py38selects	`52% <0%> (+<1%)`	⬆️
py39dnspython1	`51% <0%> (+<1%)`	⬆️
py39epolls	`52% <0%> (+<1%)`	⬆️
py39poll	`52% <0%> (+<1%)`	⬆️
py39selects	`52% <0%> (+<1%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

itamarst

Good catch! I wonder if this is the reason for the some of the weird TLS shutdown edge cases I've seen.

4383 · 2024-01-16T14:06:12Z

Good catch! I wonder if this is the reason for the some of the weird TLS shutdown edge cases I've seen.

No idea, my discover is pure serendipity...

* Update requirements from branch 'master' to 08f829d8375b4059af365191e0907069be9fb739 - Update eventlet to 0.35.0 0.35.0 ====== eventlet/eventlet#897 * [fix] fix truncate size nullable eventlet/eventlet#789 * [fix] Handle transport endpoint shutdown in conditions eventlet/eventlet#884 * [fix] Rework reject_bad_requests option eventlet/eventlet#890 * [fix] Fix NameError introduced by #826 eventlet/eventlet#890 * [feature] Support awaiting GreenThread in an `async def` context eventlet/eventlet#889 * [feature] Asyncio hub support for Python 3.7 to 3.9 eventlet/eventlet#886 * [fix] Fix bad exceptions handlings eventlet/eventlet#883 * [feature] Support using asyncio coroutines from inside greenlets eventlet/eventlet#877 * [removal] Remove deprecated CGIHTTPServer and SimpleHTTPServer eventlet/eventlet#881 * [feature] Add an asyncio hub for eventlet eventlet/eventlet#870 0.34.3 ====== eventlet/eventlet#875 * Fix security issue in the wsgi module related to RFC 9112 eventlet/eventlet#826 * Fix segfault, a new approach for greening existing locks eventlet/eventlet#866 * greendns: fix getaddrinfo parameter name eventlet/eventlet#809 * Fix deprecation warning on ssl.PROTOCOL_TLS eventlet/eventlet#872 * Pytests, fix error at teardown of TestGreenSocket.test_full_duplex eventlet/eventlet#871 * Skip test which uses Py cgi module eventlet/eventlet#865 * Drop old code based on python < 3.7.34.2 ====== eventlet/eventlet#861 * Allowing inheritance of GreenSSLSocket without overriding the __new_ method eventlet/eventlet#796 * [bug] Fix broken API related to `__version__` removal eventlet/eventlet#859 * [doc] Fix pypi broken link eventlet/eventlet#857 0.34.1 ====== eventlet/eventlet#842 * [bug] Fix memory leak in greendns eventlet/eventlet#810 * [infra] Fix OIDC authentication failure eventlet/eventlet#855 * [bug] Ignore asyncore and asynchat for Python 3.12+ eventlet/eventlet#804 0.34.0 (Not released on Pypi but landed with 0.34.1) ==================================================== * Dropped support for Python 3.6 and earlier. * Fix Python 3.13 compat by adding missing attibute '_is_main_interpreter' eventlet/eventlet#847 * Add support of Python 3.12 eventlet/eventlet#817 * Drop unmaintained and unused stdlib tests eventlet/eventlet#820 * Fix tests and CI for Python 3.7 and higher eventlet/eventlet#831 and eventlet/eventlet#832 * Stop claiming to create universal wheels eventlet/eventlet#841 * Fix green logging locks for Python versions <= 3.10 eventlet/eventlet#754 Change-Id: I909be1d1812eaed574525866dbc123083684571d

0.35.0 ====== eventlet/eventlet#897 * [fix] fix truncate size nullable eventlet/eventlet#789 * [fix] Handle transport endpoint shutdown in conditions eventlet/eventlet#884 * [fix] Rework reject_bad_requests option eventlet/eventlet#890 * [fix] Fix NameError introduced by #826 eventlet/eventlet#890 * [feature] Support awaiting GreenThread in an `async def` context eventlet/eventlet#889 * [feature] Asyncio hub support for Python 3.7 to 3.9 eventlet/eventlet#886 * [fix] Fix bad exceptions handlings eventlet/eventlet#883 * [feature] Support using asyncio coroutines from inside greenlets eventlet/eventlet#877 * [removal] Remove deprecated CGIHTTPServer and SimpleHTTPServer eventlet/eventlet#881 * [feature] Add an asyncio hub for eventlet eventlet/eventlet#870 0.34.3 ====== eventlet/eventlet#875 * Fix security issue in the wsgi module related to RFC 9112 eventlet/eventlet#826 * Fix segfault, a new approach for greening existing locks eventlet/eventlet#866 * greendns: fix getaddrinfo parameter name eventlet/eventlet#809 * Fix deprecation warning on ssl.PROTOCOL_TLS eventlet/eventlet#872 * Pytests, fix error at teardown of TestGreenSocket.test_full_duplex eventlet/eventlet#871 * Skip test which uses Py cgi module eventlet/eventlet#865 * Drop old code based on python < 3.7.34.2 ====== eventlet/eventlet#861 * Allowing inheritance of GreenSSLSocket without overriding the __new_ method eventlet/eventlet#796 * [bug] Fix broken API related to `__version__` removal eventlet/eventlet#859 * [doc] Fix pypi broken link eventlet/eventlet#857 0.34.1 ====== eventlet/eventlet#842 * [bug] Fix memory leak in greendns eventlet/eventlet#810 * [infra] Fix OIDC authentication failure eventlet/eventlet#855 * [bug] Ignore asyncore and asynchat for Python 3.12+ eventlet/eventlet#804 0.34.0 (Not released on Pypi but landed with 0.34.1) ==================================================== * Dropped support for Python 3.6 and earlier. * Fix Python 3.13 compat by adding missing attibute '_is_main_interpreter' eventlet/eventlet#847 * Add support of Python 3.12 eventlet/eventlet#817 * Drop unmaintained and unused stdlib tests eventlet/eventlet#820 * Fix tests and CI for Python 3.7 and higher eventlet/eventlet#831 and eventlet/eventlet#832 * Stop claiming to create universal wheels eventlet/eventlet#841 * Fix green logging locks for Python versions <= 3.10 eventlet/eventlet#754 Change-Id: I909be1d1812eaed574525866dbc123083684571d

JerryFuGuan · 2024-09-18T02:03:17Z

This commit changes the previous behavior when an EPIPE exception occurs.

4383 · 2024-09-18T07:01:37Z

This commit changes the previous behavior when an EPIPE exception occurs.

Well, the way these exceptions were designed originally (before this patch), made them not able to work as expected, so the previous behavior was simply to do nothing when EPIPE exception occurs...

JerryFuGuan · 2024-09-18T07:24:46Z

This commit changes the previous behavior when an EPIPE exception occurs.

Well, the way these exceptions were designed originally (before this patch), made them not able to work as expected, so the previous behavior was simply to do nothing when EPIPE exception occurs...

When subprocess.Popen.compunicate is used in the service code, the code enters an infinite loop in the os.write function and the program is suspended. Previously, we would go to the first exception handling, raise, and then exit the function, because get_errno(e)==errno.EPIPE != errno.EAGAIN. Or return an empty string like the current os.read function can also exit the function.

4383 · 2024-09-18T08:32:16Z

Would you like to propose a PR to fix it?

Fix bad exceptions handlings

eed4bb0

This patch fix 2 conditions that are badly designed. They can't work. In all case the exception will be catched by the first except and then the second one will be all the time ignored.

4383 requested a review from itamarst January 16, 2024 10:39

4383 added bug design labels Jan 16, 2024

itamarst approved these changes Jan 16, 2024

View reviewed changes

4383 merged commit 81d4359 into eventlet:master Jan 16, 2024

4383 mentioned this pull request Jan 18, 2024

[release] eventlet 0.35.0 #897

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bad exceptions handlings#883

Fix bad exceptions handlings#883
4383 merged 1 commit intoeventlet:masterfrom
4383:fix-bad-exceptions

4383 commented Jan 16, 2024 •

edited

Loading

Uh oh!

codecov bot commented Jan 16, 2024 •

edited

Loading

Uh oh!

itamarst left a comment

Uh oh!

4383 commented Jan 16, 2024

Uh oh!

JerryFuGuan commented Sep 18, 2024

Uh oh!

4383 commented Sep 18, 2024

Uh oh!

JerryFuGuan commented Sep 18, 2024

Uh oh!

4383 commented Sep 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

4383 commented Jan 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

itamarst left a comment

Choose a reason for hiding this comment

Uh oh!

4383 commented Jan 16, 2024

Uh oh!

JerryFuGuan commented Sep 18, 2024

Uh oh!

4383 commented Sep 18, 2024

Uh oh!

JerryFuGuan commented Sep 18, 2024

Uh oh!

4383 commented Sep 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

4383 commented Jan 16, 2024 •

edited

Loading

codecov bot commented Jan 16, 2024 •

edited

Loading