feat(layered): relax overly restrictive static bounds

[martintmk]

There are few places where the 'static requirement is unnecessarily enforced.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (5593a3e) to head (509f52f).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #373   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         223      223           
  Lines       16065    16065           
=======================================
  Hits        16065    16065           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR relaxes some generic bounds on layered::Execute::new and centralizes IO error recoverability classification into the recoverable crate, replacing an internal helper previously living in http_extensions.

Changes:

• Relax Execute::new bounds by removing 'static requirements from In, Out, and the returned Future.
• Add impl From<std::io::ErrorKind> for recoverable::RecoveryInfo (with tests) to provide a shared IO retry classification.
• Remove http_extensions’ internal resilience module and switch HttpError’s IO recovery to the new RecoveryInfo::from(error.kind()) path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
crates/recoverable/src/lib.rs	Adds the new io module to compile in IO-to-RecoveryInfo conversions.
crates/recoverable/src/io.rs	Introduces IO ErrorKind → RecoveryInfo classification and corresponding unit tests.
crates/layered/src/execute.rs	Relaxes overly restrictive 'static bounds for Execute::new generics.
crates/http_extensions/src/resilience.rs	Removes the now-duplicated IO recoverability helper.
crates/http_extensions/src/lib.rs	Drops the internal resilience module from the crate module list.
crates/http_extensions/src/error.rs	Updates IO error recovery classification to use RecoveryInfo::from(error.kind()).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sandersaares]

I suggest adding some tests with non-'static data. I see that some bounds were removed but I see no evidence it actually works.

[sandersaares]

I see a bunch of tests added now but aren't those all still 'static? Am I missing the non-'static references somewhere?

[martintmk]

I see a bunch of tests added now but aren't those all still 'static? Am I missing the non-'static references somewhere?

The explicit type annotation makes it clear that we are dealing with non-static strings:

    let context: ResilienceContext<&str, &str> = ResilienceContext::new(&clock);

And just to make sure, I did update the "references.cs" composite test:

    let input = "hello".to_string();
    let service = stack.into_service();
    let output = service.execute(&input).await;

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sandersaares]

The explicit type annotation makes it clear that we are dealing with non-static strings:

I do not see any non-'static lifetimes listed in the code you pasted, though. Am I missing something?

And just to make sure, I did update the "references.cs" composite test:

OK, that pattern does look believable, though.

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

crates/layered/src/dynamic.rs:60

• The impl bounds were relaxed to allow non-'static In/Out, but DynamicService still hard-codes DynService<'static, In, Out>. If the lifetime parameter on DynService represents the returned future’s lifetime (a common pattern for boxed futures), this effectively reintroduces a 'static requirement and can make the API misleading (bounds say it’s allowed, representation still forces 'static). Consider either (a) restoring In: 'static, Out: 'static bounds on DynamicService/DynamicServiceExt to match the representation, or (b) changing DynamicService to store a higher-ranked or non-'static dyn service (e.g., for<'a> DynService<'a, In, Out>) so the relaxed bounds are actually realizable.

pub struct DynamicService<In, Out>(Arc<DynService<'static, In, Out>>);

impl<In: Send, Out: Send> DynamicService<In, Out> {
    pub(crate) fn new<T>(strategy: T) -> Self
    where
        T: Service<In, Out = Out> + Send + Sync + 'static,
    {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[afoxman]

Thanks for refining the lifetimes all over the crate!

[afoxman]

Suggested change

	struct OnInput<In>(Arc<dyn for<'a> Fn(&'a In) + Send + Sync>);
	struct OnInput<'a, In>(Arc<dyn Fn(&'a In) + Send + Sync>);

I think something like this will get you closer, but the hang-up is that the closure lifetime is much longer than &In... and the closure could borrow and hold using the ref.

A stateless function, on the other hand, can't do that. So it solves the problem, but takes away closures. And users who need stateful calls would have to put their "context" data in In which is gross. Sorry, all I have is bad news :|

A cheaper, backward compatible fix would be to document that this isn't supported :)

[martintmk]

I am starting to realize that this indeed might not be possible with current Rust :(

[martintmk]

currently, we haven't found a way to make this work with references without poisoning the service lifetime. The go-to solution is to use owned types for now.