Expand params with anonymous lifetime is incompatible with 'static lifetime requirement of paginator

[seanpianka]

After upgrading from 0.14 to 0.15 and attempting to use the new paginator, it seems that the expand parameters with an anonymous lifetime (e.g. params.expand = ... where ... is &[&str]) are incompatible with List<Customer>::paginate.

error[E0759]: `expand` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
   --> crates/my-cool-project/src/lib.rs:224:67
    |
223 |     #[tracing::instrument(err, skip(self))]
    |     --------------------------------------- ...is used here...
224 |     pub async fn find_customer(&self, with_query: &CustomerQuery, expand: &[&str]) -> Result<Customer> {
    |                                                                   ^^^^^^  ------- this data with an anonymous lifetime `'_`...
    |                                                                   |
    |                                                                   ...is used here...
...
264 |                                 .next(&self.client())
    |                                  ---- ...and is required to live as long as `'static` here
    |
note: `'static` lifetime requirement introduced by this bound
   --> /Users/sean/.cargo/git/checkouts/async-stripe-2181100b8e3e802e/d6040d7/src/params.rs:224:39
    |
224 |         P: Clone + Serialize + Send + 'static + std::fmt::Debug,
    |                                       ^^^^^^^

Any advice on how to proceed? I suppose I could leak the expand parameters to satisfy a static lifetime, but it's a bit peculiar. Alternatively, I suppose the arguments to expand could always be &'static str.

[arlyon]

Hi!

Dynamic expand values is something I didn't consider in the original design but we could probably support it if needed. Can you describe a little more about how you are building the parameter? I've personally only set them statically per request, but I realise now that that use-case is obviously a little inflexible.

Having done some digging, if we were to support this, we would run into some issues (as I understand it). The lifetime of the expand param eventually makes its way into a Box::pin future, and the 'static lifetime is needed because of this so that the compiler knows that the lifetime of the expand params is greater than that of the future. This will remain the case until GATs are stabilized. Some more context is available here: https://www.reddit.com/r/rust/comments/jsxtsz/lifetime_trouble_with_traits_and_boxpin/

I believe dropping the blocking client may also allow us to achieve the same thing, as we will no longer need the Result type nor the ok and err functions.

[arlyon]

That said, having messed around with this further, the issue occurs for any fields that borrow a value which means that paginating with any borrowed params is broken. Not ideal at all. I am going to make some time to figure out how to resolve this, ideas welcome. The obvious one is to have list params take ownership of values so that the lifetimes are simplified.

This primarily affects the expand field but there are a small number of fields in addition to those where a 'static lifetime is very awkward, such as email on ListCustomers

[arlyon]

For some more context, this issue appeared while trying to fix pagination, because previously pagination completely ignored any params given entirely so I'd say this is at least a step up from that but clearly there is a ways to go.

[seanpianka]

Hey there, I appreciate the fast response. I'll work around this by making the input parameters 'static, thanks for the guidance! There's no particular rush from me to support this change, as what you've suggested sounds reasonable enough.

[seanpianka]

I suppose the underlying issue here is not that the expand params need to be dynamic, it is that the field of params ends with a 'static lifetime bound in order to paginate. In my case, this is the error:

error[E0521]: borrowed data escapes outside of associated function
   --> crates/my-cool-crate/src/lib.rs:227:33
    |
224 |     pub async fn find_customer(&self, with_query: &CustomerQuery) -> Result<Customer> {
    |                                       ----------  - let's call the lifetime of this reference `'1`
    |                                       |
    |                                       `with_query` is a reference that is only valid in the associated function body
...
227 |             params.email = Some(email.as_str());
    |                                 ^^^^^^^^^^^^^^
    |                                 |
    |                                 `with_query` escapes the associated function body here
    |                                 argument requires that `'1` must outlive `'static`

This works fine when I'm trying list customers:

    pub async fn find_customer(&self, with_query: &CustomerQuery) -> Result<Customer> {
        let mut params = ListCustomers::new();
        if let CustomerQuery::ByEmailAndId(.., email) = with_query {
            params.email = Some(email.as_str());
        }
        let mut customers =
            Customer::list(&self.client(), &params)
                .await
                .map_err(|e| StripeClientError::StripeError {
                    cause: format!("failed to find customer: {:?}", e),
                })?;
        # ...

However, later in the function when paginating through the results, the above issue appears when re-using the same params instance:

                    # ...
                    if customers.has_more {
                        let customer_pagination =
                            customers
                                .paginate(params.clone())
                                .next(&self.client())
                                .await
                                .map_err(|e| StripeClientError::StripeError {
                                    cause: format!("failed to continue paginated listing: {:?}", e),
                                })?;
                        customers = customer_pagination.page;
                    }

...is when I run into the above issue where the params.email field is made to require a 'static' lifetime.

I agree that the expand parameters rarely need to be dynamic values, however the params here certainly should be capable of allowing this.

Any more of your guidance here would be appreciated!

[arlyon]

Until I sort out wiring these lifetimes up correctly (thanks again for the report, this is a glaring error) then the suggestions is either downgrade to 0.14 (which has the pagination bug) or leak which is not ideal. Sorry for leaving you in this position, I will prioritise getting a fix out when I am back home.

edit: I had an initial go at threading through the lifetimes but have hit the GAT-limited problem hinted to above. As I see it, in the general case, there is no way to express to the rust compiler that the lifetime of the internal borrow of a generic type should be constrained to some value, so we run into the above issue where rust doesn't know about the lifetime bounds of the generic type, and so can't ensure that the borrows' lifetime is greater than that of the future.

[arlyon]

I have a fix for this (finally). I am going to rebase and merge it this weekend :)

Now all params lifetimes are propagated through the requests properly which allows borrowed params to be used as expected. Will update once I merge.