Skip to content

Commit

Permalink
test_coalescing: improve error handling
Browse files Browse the repository at this point in the history
`test_coalescing` spawns a lot of tokio tasks, each of which
runs a query on the database.
If a query fails, the task will panic because of the `unwrap()`.

The problem is that all of the tasks are joined using `join_all`,
which doesn't check for errors in the joined tasks.
For example, this piece of code prints "Ok!" and the program finishes successfuly:
```rust
let mut tasks = Vec::new();
for _i in 0..4 {
    tasks.push(tokio::spawn(async { panic!("Panic!") }));
}

futures::future::join_all(tasks).await;

println!("Ok!");
```
There are some error messages in the output, but the important thig is
that they don't stop the control flow.

To fix it, let's use `try_join_all` instead of `join_all`.
`try_join_all` will check whether the task panicked or not,
and return an error if there was a panic.

I manually tested that it works by inserting a panic!()
after `conn.query()`.

Let's also make sure that the result of join_all doesn't
contain any `Result<>`, as it could hide the errors.
The result of `join_all` should be just a `Vec<>` of unit values.

Refs: scylladb#828

Signed-off-by: Jan Ciolek <[email protected]>
  • Loading branch information
cvybhu committed Oct 19, 2023
1 parent 2641d1b commit 1136ee2
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions scylla/src/transport/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2026,16 +2026,16 @@ mod tests {
async move {
conn.query(&q, (j, vec![j as u8; j as usize]), None)
.await
.unwrap()
.unwrap();
}
});
futures::future::join_all(futs).await;
let _joined: Vec<()> = futures::future::join_all(futs).await;
}));

tokio::task::yield_now().await;
}

futures::future::join_all(futs).await;
let _joined: Vec<()> = futures::future::try_join_all(futs).await.unwrap();

// Check that everything was written properly
let range_end = arithmetic_sequence_sum(NUM_BATCHES);
Expand Down

0 comments on commit 1136ee2

Please sign in to comment.