Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interface_bitcoin_cli.py fails intermittently #18684

Closed
maflcko opened this issue Apr 17, 2020 · 9 comments · Fixed by #18691
Closed

interface_bitcoin_cli.py fails intermittently #18684

maflcko opened this issue Apr 17, 2020 · 9 comments · Fixed by #18691
Labels

Comments

@maflcko
Copy link
Member

maflcko commented Apr 17, 2020

https://travis-ci.org/github/bitcoin/bitcoin/jobs/676156661#L6349

 test  2020-04-17T14:09:31.865000Z TestFramework (ERROR): 
  Called Process failed with 'error: Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set.  See -rpcpassword and -stdinrpcpass.  Configuration file: (/home/travis/build/bitcoin/bitcoin/ci/scratch/test_runner/test_runner_₿_🏃_20200417_132645/interface_bitcoin_cli_109/node0/bitcoin.conf) 

cc @jonatack

@maflcko maflcko added the Bug label Apr 17, 2020
@maflcko
Copy link
Member Author

maflcko commented Apr 17, 2020

The failure is in the line with -rpcwait, you probably need wait_for_rpc_connection

@brakmic
Copy link
Contributor

brakmic commented Apr 17, 2020

Same problem with my PR: https://travis-ci.org/github/bitcoin/bitcoin/jobs/676136137

@jonatack
Copy link
Member

jonatack commented Apr 17, 2020

Same problem with my PR: https://travis-ci.org/github/bitcoin/bitcoin/jobs/676136137

Are you able to reproduce locally?
Even valgrind test/functional/interface_bitcoin_cli.py -l debug runs fine for me.

@brakmic
Copy link
Contributor

brakmic commented Apr 17, 2020

Same problem with my PR: https://travis-ci.org/github/bitcoin/bitcoin/jobs/676136137

Are you able to reproduce locally?

Sadly, I'm not. All test pass, no errors whatsoever.

@brakmic
Copy link
Contributor

brakmic commented Apr 17, 2020

@jonatack

However, there's one thing I just did that provoked the same error:

I changed setup_clean_chain to False.

self.setup_clean_chain = False

Then I got this:

./test/functional/interface_bitcoin_cli.py                                          (3s 428ms)  
2020-04-17T16:59:26.097000Z TestFramework (INFO): Initializing test directory /var/folders/7q/4ffytzk562dd2ky4bfg9_w7h0000gn/T/bitcoin_func_test_6h67eta3
2020-04-17T16:59:28.940000Z TestFramework (INFO): Compare responses from getwalletinfo RPC and `bitcoin-cli getwalletinfo`
2020-04-17T16:59:29.002000Z TestFramework (INFO): Compare responses from getblockchaininfo RPC and `bitcoin-cli getblockchaininfo`
2020-04-17T16:59:29.024000Z TestFramework (INFO): Test -stdinrpcpass option
2020-04-17T16:59:29.043000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/Users/brakmic/projects/bitcoin-dev/btc-global-ctx/test/functional/test_framework/test_framework.py", line 112, in main
    self.run_test()
  File "./test/functional/interface_bitcoin_cli.py", line 45, in run_test
    assert_equal(BLOCKS, self.nodes[0].cli('-rpcuser=%s' % user, '-stdinrpcpass', input=password).getblockcount())
  File "/Users/brakmic/projects/bitcoin-dev/btc-global-ctx/test/functional/test_framework/util.py", line 46, in assert_equal
    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(101 == 301)
2020-04-17T16:59:29.123000Z TestFramework (INFO): Stopping nodes
2020-04-17T16:59:29.584000Z TestFramework (WARNING): Not cleaning up dir /var/folders/7q/4ffytzk562dd2ky4bfg9_w7h0000gn/T/bitcoin_func_test_6h67eta3
2020-04-17T16:59:29.585000Z TestFramework (ERROR): Test failed. Test logging available at /var/folders/7q/4ffytzk562dd2ky4bfg9_w7h0000gn/T/bitcoin_func_test_6h67eta3/test_framework.log
2020-04-17T16:59:29.586000Z TestFramework (ERROR): Hint: Call /Users/brakmic/projects/bitcoin-dev/btc-global-ctx/test/functional/combine_logs.py '/var/folders/7q/4ffytzk562dd2ky4bfg9_w7h0000gn/T/bitcoin_func_test_6h67eta3' to consolidate all logs

Which could mean that this fails when it runs on a touched chain.

--EDIT: forgot to mention that this is only a speculation of mine.

@bitcoin bitcoin deleted a comment from jasdeep9899 Apr 17, 2020
@jonatack
Copy link
Member

The issue isn't from running too slowly: I added timeouts of up to 90 seconds before and after the call and the test doesn't fail.

         # Verify failure without -rpcwait.
         assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('getblockcount').echo)
         # Verify success using -rpcwait.
+        import time; time.sleep(90)
         assert_equal(BLOCKS, self.nodes[0].cli('-rpcwait', 'getblockcount').send_cli())
+        time.sleep(90)
         self.nodes[0].wait_for_rpc_connection()

AFAICT it's failing because failedToGetAuthCookie is set to true at bitcoin-cli.cpp::L352 and triggering the throw at bitcoin-cli.cpp::L398.

@brakmic
Copy link
Contributor

brakmic commented Apr 17, 2020

The issue isn't from running too slowly: I added timeouts of up to 90 seconds before and after the call and the test doesn't fail.

         # Verify failure without -rpcwait.
         assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('getblockcount').echo)
         # Verify success using -rpcwait.
+        import time; time.sleep(90)
         assert_equal(BLOCKS, self.nodes[0].cli('-rpcwait', 'getblockcount').send_cli())
+        time.sleep(90)
         self.nodes[0].wait_for_rpc_connection()

AFAICT it's failing because failedToGetAuthCookie is set to true at bitcoin-cli.cpp::L352 and triggering the throw at bitcoin-cli.cpp::L398.

Interesting!

Maybe in future, if my PR regarding factor option for timeouts gets accepted, we could add such an option for this test as well?

@jonatack
Copy link
Member

Tempted to propose something like this, as I can't reproduce the failure locally:

         # Verify success using -rpcwait.
+        time.sleep(1)
         assert_equal(BLOCKS, self.nodes[0].cli('-rpcwait', 'getblockcount').send_cli())

@brakmic
Copy link
Contributor

brakmic commented Apr 17, 2020

Tempted to propose something like this, as I can't reproduce the failure locally:

         # Verify success using -rpcwait.
+        time.sleep(1)
         assert_equal(BLOCKS, self.nodes[0].cli('-rpcwait', 'getblockcount').send_cli())

Well, I am not a python expert, but wouldn't be better to have some kind of async/wait or JS-like promise function (not sure what's the proper, pythonic way of doing such things).
thread.sleep feels kinda hard, like a stop sign.

sidhujag pushed a commit to syscoin/syscoin that referenced this issue Apr 20, 2020
…work for rpcwait tests

92fe537 test: fix intermittent race condition in interface_bitcoin_cli.py (Jon Atack)
c648e63 test: add wait_for_cookie_credentials() to test framework (Jon Atack)

Pull request description:

  This PR adds a `wait_for_cookie_credentials()` method to the test framework and calls it before the `-rpcwait` tests, to avoid an intermittent race condition on the CI run with Valgrind where the cookie file isn't written yet when the CLI call with `-rpcwait` arrives to `get_auth_cookie()`.

  To reproduce/test, build with
  ```diff
  diff --git a/src/httprpc.cpp b/src/httprpc.cpp
  index 60c4d06f12..3dd06c4758 100644
  --- a/src/httprpc.cpp
  +++ b/src/httprpc.cpp
  @@ -291,6 +291,7 @@ static bool InitRPCAuthentication()
   bool StartHTTPRPC()
   {
       LogPrint(BCLog::RPC, "Starting HTTP RPC server\n");
  +    UninterruptibleSleep(std::chrono::seconds{11});
       if (!InitRPCAuthentication())
  ```
  then run the test normally and with valgrind
  ```
  test/functional/interface_bitcoin_cli.py -l debug
  valgrind test/functional/interface_bitcoin_cli.py -l debug
  ```
  Thanks to Marco Falke for all the help. Closes bitcoin#18684.

Top commit has no ACKs.

Tree-SHA512: 1b76635b5b1d6b05138affef7ab788aa3bc3fc75b0c69ba778ecdf81063cfe02a8dd7667cfd63a6c6e19b2dac47d7a8b755e334d8af5c0ab9d4026808ee96c83
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants