test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2) #18633

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2004-qaResetTake2 changing 1 files +3 −3

MarcoFalke commented at 2:21 PM on April 14, 2020: member

actually (?) fix #18561

See most recent traceback https://travis-ci.org/github/bitcoin/bitcoin/jobs/674668692#L7062

I believe the reason the error is still there is that ConnectionResetError is derived from OSError:

ConnectionResetError(ConnectionError(OSError))

And IOError is an alias for OSError since python 3.3, see https://docs.python.org/3/library/exceptions.html#IOError

So fix that by renaming IOError to the alias OSError and move the less specific catch clause down a few lines.
MarcoFalke force-pushed on Apr 14, 2020
DrahtBot added the label Tests on Apr 14, 2020

in test/functional/test_framework/test_node.py:239 in fa864e35fc outdated

 233 | @@ -237,6 +234,9 @@ def wait_for_rpc_connection(self):
 234 |                  # This might happen when the RPC server is in warmup, but shut down before the call to getblockcount
 235 |                  # succeeds. Try again to properly raise the FailedToStartError
 236 |                  pass
 237 | +            except OSError as e:
 238 | +                if e.errno != errno.ECONNREFUSED:  # Port not yet open?
 239 | +                    raise  # unknown IO error

robot-visions commented at 6:21 PM on April 18, 2020:

Super nit: Change comment to say unknown OS error?

MarcoFalke commented at 6:59 PM on April 18, 2020:

Thanks, fixed

robot-visions commented at 6:25 PM on April 18, 2020: contributor

Thanks for looking into this!

Although I don't understand the root cause of the failing build, I do agree that except IOError will catch a ConnectionResetError, and it makes sense to put the more specific case first.
test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2) fa03713e13
MarcoFalke force-pushed on Apr 18, 2020
MarcoFalke commented at 6:59 PM on April 18, 2020: member

Addressed feedback by @robot-visions
jonatack commented at 8:58 PM on April 18, 2020: member

ACK fa03713e133e3017112fdd5c278e0c8643054578

Noting eventually for follow-ups, that there are instances of IOError in feature_assumevalid.py, p2p_invalid_messages.py, test_framework/mininode.py, and test_framework/socks5.py.
DrahtBot commented at 4:33 AM on April 19, 2020: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #18691 (test: add wait_for_cookie_credentials() to framework for rpcwait tests by jonatack)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
MarcoFalke merged this on Apr 19, 2020
MarcoFalke closed this on Apr 19, 2020
MarcoFalke deleted the branch on Apr 19, 2020
sidhujag referenced this in commit 0d241c80d4 on Apr 20, 2020
jasonbcox referenced this in commit c529d04931 on Jul 30, 2020
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#18561 test: Properly raise FailedToStartError when rpc shutdown before warmup finished

test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2) #18633

Conflicts