The rpc handler may be unregistered when http server haven't been closed yet. So it may be allowable to get -342 non-JSON HTTP response with \'%i %s\' from server (503 Service Unavailable)
See https://ci.appveyor.com/project/DrahtBot/bitcoin/build/master.2001. It shows "Rejecting request while shutting down" between "RPC stopped" and "Stopped HTTP server"
Concept ACK
I've seen this in AppVeyor. Thanks for addressing!
Code change seems reasonable, but I don't understand the issue.
What would cause the RPC handler to become unregistered while the test framework is in wait_for_rpc_connection? Why would the nodes be getting shut down if the test framework isn't shutting them down?
186 | @@ -187,7 +187,11 @@ def wait_for_rpc_connection(self): 187 | if e.errno != errno.ECONNREFUSED: # Port not yet open? 188 | raise # unknown IO error 189 | except JSONRPCException as e: # Initialization phase 190 | - if e.error['code'] != -28: # RPC in warmup? 191 | + if e.error['code'] == -28: # RPC in warmup? 192 | + pass 193 | + elif e.error['code'] == -342: # RPC handler unregistered, but http server haven't been stopped yet. 194 | + raise FailedToStartError(self._node_msg(e.error['message']))
Maybe just pass this on as well? This way we'd make sure to always have an exit code as opposed to randomly this message or an exit code due to the racyness.
Would it be good to call self.process.wait() here and include the exit code in the exception, so this is more similar to the previous FailedToStartError exception above?
Sure, fixed
I would prefer @MarcoFalke's method, then we don't have to raise it here again.
@ryanofsky If the node can't initialize itself shuts down itself without having the test framework shut it down.
186 | @@ -187,7 +187,11 @@ def wait_for_rpc_connection(self): 187 | if e.errno != errno.ECONNREFUSED: # Port not yet open? 188 | raise # unknown IO error 189 | except JSONRPCException as e: # Initialization phase 190 | - if e.error['code'] != -28: # RPC in warmup? 191 | + if e.error['code'] == -28: # RPC in warmup? 192 | + pass 193 | + elif e.error['code'] == -342: # RPC handler unregistered, but http server haven't been stopped yet.
I think this comment is confusing without mentioning why the server is shutting down. Maybe change it to something like:
# Service unavailable, RPC server started but is shutting down due to error
Comment added
@ryanofsky If the node can't initialize itself shuts down itself without having the test framework shut it down.
Thanks. I didn't realize assert_start_raises_init_error would wait for RPCs to be ready. But I guess it needs to in order to call shut down the node when the error doesn't trigger.
utACK 62c304ea481d474bc87d950e21907b8b05134fe7
utACK 62c304ea481d474bc87d950e21907b8b05134fe7