TypeError: exceptions must be old-style classes or derived from BaseException, not NoneType

Bug #1181146 reported by Sergio Rubio
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Medium
Samuel Merritt

Bug Description

proxy_logging middleware seems to have some kind of race condition when re-raising an exception under high load:

May 16 16:19:44 swift-proxy-02 proxy-server Error: exceptions must be old-style classes or derived from BaseException, not NoneType: #012Traceback (most recent call last):#012 File "/usr/lib/python2.7/dist-packages/swift/common/middleware/catch_errors.py", line 36, in handle_request#012 resp = self._app_call(env)#012 File "/usr/lib/python2.7/dist-packages/swift/common/wsgi.py", line 275, in _app_call#012 resp = self.app(env, self._start_response)#012 File "/usr/lib/python2.7/dist-packages/swift/common/middleware/bvox_lbnode.py", line 21, in __call__#012 return self.app(environ, custom_start_response)#012 File "/usr/lib/python2.7/dist-packages/swift/common/middleware/healthcheck.py", line 57, in __call__#012 return self.app(env, start_response)#012 File "/usr/lib/python2.7/dist-packages/swift/common/middleware/proxy_logging.py", line 250, in __call__#012 raise#012TypeError: exceptions must be old-style classes or derived from BaseException, not NoneType (txn: txa80ff8908505413ea7785bf5049343a6) (client_ip: XXXXXX)

and the problematic code:

        try:
            iterable = self.app(env, my_start_response)
        except Exception:
            req = Request(env)
            status_int = status_int_for_logging(start_status=500)
            self.log_request(
                req, status_int, input_proxy.bytes_received, 0,
                time.time() - start_time)
            raise
        else:
            return iter_response(iterable)

Still don't know why this is happening but this (related to eventlet) explanation sounds reasonable:

http://stackoverflow.com/a/9788109

I'm no python guru so I could be completely wrong.

Revision history for this message
Samuel Merritt (torgomatic) wrote :

Yep, seems like it's legit. If the logging does socket IO, then it may cause a greenlet switch, and I don't think exc_info is preserved with the current greenlet as it's not on the stack.

Changed in swift:
assignee: nobody → Samuel Merritt (torgomatic)
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (master)

Fix proposed to branch: master
Review: https://review.openstack.org/29621

Changed in swift:
status: Confirmed → In Progress
Revision history for this message
Edward Hope-Morley (hopem) wrote :

This problem is caused by the follwoing bug in python-eventlet:

https://bitbucket.org/eventlet/eventlet/issue/149/yield-in-except-clause-with-wilcard-raise

A fix has been comitted but is waiting for the next release. There is a workaround already commonly used in Openstack:

nova.openstack.common.excutils.save_and_reraise_exception()

The number of except clauses that contain a yield should be minimal so it should not be necessary to path them all. Probably best to follow same path as other services i.e. use save_and_reraise_exception() and wait for eventlet patch to be released.

Revision history for this message
Samuel Merritt (torgomatic) wrote :

Agreed; the vast majority of bare "raise" calls in Swift are checking some attribute of the error (e.g. "err.errno in (X, Y, Z)" or "'some string' in err.message") to swallow some subset of the errors. There's only a handful that are logging right now, so those are the ones that should get patched up.

Hopefully the Eventlet folks release soon...

Revision history for this message
Edward Hope-Morley (hopem) wrote :

A fix for this has now been released in python-eventlet 0.13. Hopefully it will land in all distros a.s.a.p

Revision history for this message
clayg (clay-gerrard) wrote :

so eventlet 0.13 is out in the wild and available for newish type distros - it's not clear if that closes this bug against swift?

Changed in swift:
status: In Progress → Incomplete
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/181566

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (master)

Reviewed: https://review.openstack.org/181566
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=90b84d3a699811a99c97ebbe4f71a14d2f76a0e5
Submitter: Jenkins
Branch: master

commit 90b84d3a699811a99c97ebbe4f71a14d2f76a0e5
Author: Tim Burke <email address hidden>
Date: Fri May 8 11:45:12 2015 -0700

    Properly re-raise exceptions in proxy_logging

    Previously, this could encounter TypeErrors, presumably because
    sys.exc_clear() was called somewhere in the block of code between
    catching the exception and re-raising.

    Related-Bug: 1181146
    Change-Id: Iadeea3f61e70bf83dc0eb063fdb27edd16f3ca32

Changed in swift:
status: Incomplete → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (feature/hummingbird)

Related fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/202227

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on swift (feature/hummingbird)

Change abandoned by Michael Barton (<email address hidden>) on branch: feature/hummingbird
Review: https://review.openstack.org/202227
Reason: Apparently I did this wrong.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (feature/hummingbird)

Related fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/202230

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (feature/hummingbird)
Download full text (72.8 KiB)

Reviewed: https://review.openstack.org/202230
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=f7cb1777e1b514b3345b9e516ed8f89ad1a4ae87
Submitter: Jenkins
Branch: feature/hummingbird

commit 51f806d3e3d3a1fcbc80d2f7d7ddbe5cc4c024c9
Author: John Dickinson <email address hidden>
Date: Tue Jul 14 20:49:08 2015 -0700

    remove Python 2.6 from the classifier

    Change-Id: I67233e9c7b69826242546bd6bd98c24b81070579

commit 278adf5c20101a191979ce1e4d6277e5f209149e
Author: Hisashi Osanai <email address hidden>
Date: Tue Jul 14 15:33:45 2015 +0900

    Make logic of unit tests responsive to the method names

    The two methods, test_authorize_succeeds_for_tenant_name_in_roles and
    test_authorize_succeeds_for_tenant_id_in_roles, have names that don't
    match what they are testing. tenant_name and tenant_id need to be
    switched.

    Change-Id: I7cb0a7d2b2111127fd5d6b55f2da6a3eadf2235d

commit 1cc3eff958fdd4fb07c2b74c52df7829d3125466
Author: Victor Stinner <email address hidden>
Date: Fri Jul 10 13:04:44 2015 +0200

    Fixes for mock 1.1

    The new release of mock 1.1 is more strict. It helped to find bugs in
    tests.

    Closes-Bug: #1473369
    Change-Id: Id179513c6010d827cbcbdda7692a920e29213bcb

commit ff192cfe5705324497a389aa2f22227d75dc0f8e
Author: janonymous <email address hidden>
Date: Wed Jul 8 18:38:22 2015 +0530

    Replace reduce and unichr , these are no longer available in py3

    * Replace reduce() with six.moves.reduce()
    * Replace unichr with six.unichr

    Change-Id: I2038e47e0a6522dd992fd2a4aeff981cf7750fe0

commit 4beceab4f4be99f14025815cf7ed4510ea77f460
Author: OpenStack Proposal Bot <email address hidden>
Date: Thu Jul 9 06:14:56 2015 +0000

    Imported Translations from Transifex

    For more information about this automatic import see:
    https://wiki.openstack.org/wiki/Translations/Infrastructure

    Change-Id: I9ff1dde06be45fc7d6c441a1e1c07221f839a9a1

commit 56ee39a7e13417203c5e1816d7a3184a07f85826
Author: Matthew Oliver <email address hidden>
Date: Thu Jul 9 15:19:32 2015 +1000

    Ring builder code clean up follow up patch

    This is a simple change that cleans up a NIT from Sam's 'stop moving
    partitions unnecessarily when overload is on' patch.

    Change-Id: I9d9f1cc23e2bb625d8e158f4d3f64e10973176a1

commit 6cafd0a4c0bb8f311fc59df580b42e801214effd
Author: Oshrit Feder <email address hidden>
Date: Wed Jul 8 15:18:22 2015 +0300

    Fix Container Sync example

    Container-sync realm uses cluster_ as a prefix to specify clusters'
    names. At use, the prefix should not be included. Fixing the examples
    and sample conf to make it clearer that only the name of the cluster
    should be passed.

    Change-Id: I2e521d86faffb59e1b45d3f039987ee023c5e939

commit 125238612f58481316db68d7087252bb7729f447
Author: Janie Richling <email address hidden>
Date: Sat Jul 4 17:08:32 2015 -0500

    Add CORS unit tests to base

    In earlier versions of swift when a request was made with an
    existing origin, but without any CORS settings in the container,
    it was possible to get an u...

tags: added: in-feature-hummingbird
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (feature/crypto)

Related fix proposed to branch: feature/crypto
Review: https://review.openstack.org/205579

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (feature/crypto)
Download full text (80.5 KiB)

Reviewed: https://review.openstack.org/205579
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=8ab46b64365b8eab80680f2562f81e8adb3032a3
Submitter: Jenkins
Branch: feature/crypto

commit 89f705e8aab144092d40a13fc4ef19ffef5f3eba
Author: OpenStack Proposal Bot <email address hidden>
Date: Thu Jul 23 06:11:27 2015 +0000

    Imported Translations from Transifex

    For more information about this automatic import see:
    https://wiki.openstack.org/wiki/Translations/Infrastructure

    Change-Id: I94cf347564cb33977f33b1e64259bcb39a8cf809

commit a65e9db8752793ec37b594dc9eca5066171825db
Author: Christian Schwede <email address hidden>
Date: Wed Jul 22 10:43:17 2015 +0000

    Removing commented out code in test/unit/account/test_backend.py

    Noticed this while reviewing another change. Looks like the test itself already
    ensures correct functionality of the reclaim() method in AccountBroker without
    the commented code, thus removing this stale code.

    Change-Id: I6a26a7591adef9fd794ca68a4e9c493d1127f93c

commit 99d052772a9585e0befdfd292fd03aefde77180a
Author: Kota Tsuyuzaki <email address hidden>
Date: Mon Jul 13 01:12:43 2015 -0700

    Fix 499 client disconnected on COPY EC object

    Currently, a COPY request for an EC object might go to fail as 499 Client
    disconnected because of the difference between destination request content
    length and actual transferred bytes.

    That is because the conditional response status and content length for
    an EC object range GET is handled at calling the response instance on
    proxy server. Therefore the calling response instance (resp()) will change
    the conditional status from 200 (HTTP_OK) to 206 (PartialContent) and will
    change the content length for the range GET.

    In EC case, sometimes Swift needs whole stored contents to decode a segment.
    It will make 200 HTTP OK response from object-server and proxy-server
    will unfortunately set whole content length to the destination content
    length and it makes the bug 1467677.

    This patch introduces a new method "fix_conditional_response" for
    swift.common.swob.Response that calling _response_iter() and cached the
    iter in the Response instance. By calling it, Swift can set correct condtional
    response any time after setting whole content_length to the response
    instance like EC case.

    Change-Id: If85826243f955d2f03c6ad395215c73daab509b1
    Closes-Bug: #1467677

commit 62ed4f81ef80440550633eaaaa962a4f9383c2d3
Author: Timur Alperovich <email address hidden>
Date: Tue Jul 14 16:56:44 2015 -0700

    Add two functional tests for delimiter.

    The first test verifies that a delimiter will trim entries beyond the
    first matching instance of delimiter (after the given matching prefix,
    if any) and squash duplicates. So, when setting the delimiter
    to "-", given blobs "test", "test-foo" and "test-bar-baz", we expect
    only "test" (no matching delim) and "test-" (trim all characters after
    the first "-", and squash duplicates).

    The second test verifies that when a prefix is provid...

tags: added: in-feature-crypto
Thierry Carrez (ttx)
Changed in swift:
milestone: none → 2.4.0
status: Fix Committed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/570477

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (master)

Reviewed: https://review.openstack.org/570477
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=7fd5a128722a4c42406d195b46edb0b0fbc3c3e1
Submitter: Zuul
Branch: master

commit 7fd5a128722a4c42406d195b46edb0b0fbc3c3e1
Author: Tim Burke <email address hidden>
Date: Thu May 24 15:44:39 2018 -0700

    Stop holding on to sys.exc_info tuples quite so much

    This used to be necessary on older eventlet, or you'd get TypeErrors
    when you went to reraise. Following eventlet 0.13.0, however, it's just
    extra code.

    For the original eventlet issue, see
    https://web.archive.org/web/20140823005223/https://bitbucket.org/eventlet/eventlet/issue/149/yield-in-except-clause-with-wilcard-raise

    Change-Id: I19ad0968a82827bdd4ef75fde9ed51f193627d6e
    Related-Bug: 1181146

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.