Skip to content

Conversation

@JoseSK999
Copy link
Contributor

@JoseSK999 JoseSK999 commented Nov 4, 2024

As with #271, floresta-cli tests can fail sometimes due to port conflicts. This should fix it.

  • Make the electrum addresses be assigned by the OS to avoid conflicts (args with port 0)
  • Similarly get an available port for the rpc server and the client, on the cli side
  • Improve panic messages

Finally we change RpcImpl::create in order to fix the error:

"Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context."

When start_http panics (e.g. when addr is already in use), its own runtime is dropped and we see this error, which hides the error that originated the panic. This is fixed by using our tokio runtime instead:

ServerBuilder::new(io)
    .event_loop_executor(runtime_handle)
    // Threads set to 1 as we have passed our multi-threaded runtime executor
    .threads(1)
    // ...

For instance, if you input --rpc-address already_in_use to florestad, you will now see a panic message with "Address already in use", but previously (not using our current tokio runtime) you'd see "Cannot drop a runtime...".

- Make the electrum addresses be assigned by the OS to avoid conflicts (args with port `0`)
- Similarly get an available port for the rpc server and the client, on the cli side
- Improve panic messages

Finally we change `RpcImpl::create` in order to fix the "Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context." error. When `start_http` panics (e.g. when addr is already in use), its own runtime is dropped and we see this error, which hides the error that originated the panic. This is fixed by using our tokio runtime instead.
@Davidson-Souza
Copy link
Member

ACK c34ac8a

@Davidson-Souza Davidson-Souza merged commit 26625b2 into vinteumorg:master Nov 5, 2024
6 checks passed
@JoseSK999 JoseSK999 deleted the fix-cli-tests branch November 5, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants