SlackApiScrollableRequest across await

[nhey]

The scroller method in trait SlackApiScrollableRequest returns a SlackApiResponseScroller which is not Send. This means you cannot paginate from inside a thread (or I am doing it wrong). The minimal example from your website, but inside a tokio thread:

use slack_morphism::*;
use slack_morphism::api::*;
use slack_morphism_hyper::*;

async fn example() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
    let hyper_connector = SlackClientHyperConnector::new();
    let client = SlackClient::new(hyper_connector);

    let token_value: SlackApiTokenValue = "xoxb-89.....".into();
    let token: SlackApiToken = SlackApiToken::new(token_value);
    let session = client.open_session(&token);

    let scroller_req: SlackApiUsersListRequest = SlackApiUsersListRequest::new().with_limit(5);

    let scroller = scroller_req.scroller();

    use futures::TryStreamExt;
    let mut items_stream = scroller.to_items_stream(&session);
    while let Some(items) = items_stream.try_next().await? {
        println!("users batch: {:#?}", items);
    }

    Ok(())
}

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
    tokio::spawn(async move {
        example().await
    }).await?
}

yields

$ cargo build
    Updating crates.io index
   Compiling slack-send v0.1.0 (/home/hey/platform/slack-send)
error: future cannot be sent between threads safely
   --> src/main.rs:28:5
    |
28  |     tokio::spawn(async move {
    |     ^^^^^^^^^^^^ future created by async block is not `Send`
    |
   ::: /home/hey/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.9.0/src/task/spawn.rs:127:21
    |
127 |         T: Future + Send + 'static,
    |                     ---- required by this bound in `tokio::spawn`
    |
    = help: the trait `std::marker::Send` is not implemented for `dyn slack_morphism::SlackApiResponseScroller<slack_morphism_hyper::SlackClientHyperConnector, CursorType = SlackCursorId, ResponseItemType = SlackUser, ResponseType = SlackApiUsersListResponse>`
note: future is not `Send` as this value is used across an await
   --> src/main.rs:19:29
    |
15  |     let scroller = scroller_req.scroller();
    |         -------- has type `Box<dyn slack_morphism::SlackApiResponseScroller<slack_morphism_hyper::SlackClientHyperConnector, CursorType = SlackCursorId, ResponseItemType = SlackUser, ResponseType = SlackApiUsersListResponse>>` which is not `Send`
...
19  |     while let Some(items) = items_stream.try_next().await? {
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ await occurs here, with `scroller` maybe used later
...
24  | }
    | - `scroller` is later dropped here

The program compiles if I mark the return value of the scroller method as Send and Sync. I deem that this is safe since all of its members are Send and Sync, but I am a rust novice.

1. Is there a reason not to mark this as Send and Sync?
2. If not, do you want me to PR the changes? (also to the extension in the hyper module).

[abdolence]

Hey @nhey , seems like it was forgotten to mark as Send, I'll check it a bit later today and will fix.

[abdolence]

Thanks for the report!

[nhey]

Thanks. I think in some places it could be marked Clone as well?
Here's my diff with regards to Send + Sync FWIW, but there may be other places:

diff --git a/src/client/src/scroller.rs b/src/client/src/scroller.rs
index 978056e..186e45a 100644
--- a/src/client/src/scroller.rs
+++ b/src/client/src/scroller.rs
@@ -48,7 +48,7 @@ where
                 ResponseType = Self::ResponseType,
                 CursorType = Self::CursorType,
                 ResponseItemType = Self::ResponseItemType,
-            > + 'b,
+            > + Send + Sync + 'b,
     >
     where
         Self: Send + Clone + Sync + 'b,
diff --git a/src/hyper/src/scroller_ext.rs b/src/hyper/src/scroller_ext.rs
index 75f8a23..d6405d0 100644
--- a/src/hyper/src/scroller_ext.rs
+++ b/src/hyper/src/scroller_ext.rs
@@ -9,7 +9,7 @@ use std::time::Duration;
 use tokio_stream::StreamExt;

 pub trait SlackApiResponseScrollerExt<SCHC, CT, RT, RIT>:
-    SlackApiResponseScroller<SCHC, CursorType = CT, ResponseType = RT, ResponseItemType = RIT>
+    SlackApiResponseScroller<SCHC, CursorType = CT, ResponseType = RT, ResponseItemType = RIT> + Send + Sync
 where
     SCHC: SlackClientHttpConnector + Send + Sync,
     RT: Send + Clone + Sync + SlackApiScrollableResponse<CursorType = CT, ResponseItemType = RIT>,
@@ -34,7 +34,7 @@ impl<SCHC, CT, RT, RIT> SlackApiResponseScrollerExt<SCHC, CT, RT, RIT>
         CursorType = CT,
         ResponseType = RT,
         ResponseItemType = RIT,
-    >
+    > + Send + Sync
 where
     SCHC: SlackClientHttpConnector + Send + Sync,
     RT: Send + Clone + Sync + SlackApiScrollableResponse<CursorType = CT, ResponseItemType = RIT>,

[abdolence]

@nhey there is no need to mark it Copy in this case I think (and I don't think it will work if you try).
However I've fixed Send + Sync in the scroller implementation and released it in v0.8.5.
Thanks for the detailed report.
By the way, I would recommend to use to_items_throttled_stream IRL, instead of just to_items_stream in your example, because you will end up really quickly with Slack rate limitation errors.