Not sure about the xml conf, advices welcome!

Can you please rebase on 6.3 since this is a bugfix?

Rebased on 6.3!

Edit: Edit: Still got a problem with the way base uris are defined.
Configuration-wise the proposed changes seem to work. But not in the field.

On a fresh install in 6.3:

# config/packages/framework.yaml
    http_client:
        scoped_clients:
            test_client:
                base_uri:
                    - 'https://jsonplaceholder.typicode.com/'
                    - 'https://jsonplaceholder.typicode.com/'
                retry_failed:
                    max_retries: 3

// in any controller
    #[Route('/', name: 'app_client')]
    public function index(HttpClientInterface $testClient): Response
    {
        dump($testClient->request('GET', '/posts/1')->toArray());
        // ...
    }

Results in a TypeError: Symfony\Component\HttpClient\ScopingHttpClient::forBaseUri(): Argument #2 ($baseUri) must be of type string, array given since the base_uri are passed when creating the ScopingHttpClient and not the RetryableHttpClient.

Any thought on how to do this? Conditional definition for both seems the most straightforward way, but I'm not sure about that

Made an attempt just as a discussion base. Works as is, but maybe there's a better way to do it or some constraints I haven't tought about.

Hey there!

Sorry for the ping, I know it's been a while, but could I interest you back in a review around here? @nicolas-grekas maybe, or someone else? This was asked in an issue so maybe we should look into it.

In the meantime it needs to be rebased on 6.4 please

Okay there have been obviously a huge mistake on my part here... Fixing asap.

Seems okay with the right destination branch. Sorry for the spam!

Test failures seem unrelated here, but any advice would be welcome!

Hey there!

Bumping this, as it's still a missing feature. Should I create a new PR to get back in the recent ones?

Hi @Tiriel, thanks for your patience.
I think I changed my mind on the topic: this looks like a new feature to me.
To built on this idea, I'm wondering if it wouldn't make more sense to allow a new base_uris option under retry_failed.
That would make everything simpler I think.

Hi @Tiriel, thanks for your patience. I think I changed my mind on the topic: this looks like a new feature to me.

That's no problem to me, I'll go and change the PR description!

To built on this idea, I'm wondering if it wouldn't make more sense to allow a new base_uris option under retry_failed. That would make everything simpler I think.

Not sure about that though. I don't know if it's a better idea or not, but I'm not sure about what we should priorize:

• consistency (keep the base_uri option where it is already defined for the scoped clients)
• intent behind the change (allow to try multiple uris in case of retry)

Both seem to have pros and cons to me, but ultimately I'll get behind the majority advice (or your, if there not much debate ;) )

Also going to rebase and target the latest branch asap

I think base_uris under retry_strategy is more flexible as it allows using scope on top to control how scoped clients behave

Then how do we deal with the base_uri option under each scoped client? Unless I miss something, it would be possible to define a base_uri in a scope client and an array of base_uris in the retry_strategy.

I see four possibilities:

• adding a base_uri under the scoped client and an array of base_uris in the retry strategy is blocked, raising an exception
• adding a base_uri under the scoped client and an array of base_uris in the retry strategy is not blocked, base uri from scoped client is kept
• adding a base_uri under the scoped client and an array of base_uris in the retry strategy is not blocked, base uris from retry strategy are kept
• adding a base_uri under the scoped client and an array of base_uris in the retry strategy is not blocked, base uri from scoped client is merged inside the array

Personnally, I think the first option (blocking) or the last (merging) are the best, with a preference for the blocking.

I would merge them: the base_uri is prepended to base_uris if any.

Hum, actually no: we'd need to make setting both base_uri and base_uris conflict, and we'd then need to compute a regular expression that "|" all base_uris so that scoped clients accepts them all in its scope (unless scope is set).

Alternatively, if we keep base_uri as allowing an array, we could make defining a retry_strategy optional (and auto-enable it then).
We also need to allow setting both a scope and an array of base_uris for completeness (both options are exclusive at the moment).

WDYT?

Alternatively, if we keep base_uri as allowing an array, we could make defining a retry_strategy optional (and auto-enable it then).

This seems like a better option to me, a bit more friendly to use

We also need to allow setting both a scope and an array of base_uris for completeness (both options are exclusive at the moment).

This seems a bit redundant. IIRC, scope makes the configuration active only on uris matching the scope. So if you set an array of base uris and the scope, the scope will simply be active on every call since the base uris matching the scope will be prepended to each call.

In this case, I haven't looked at feasability yet, but maybe we could have a unified scope/base_uri option, working differently depending on the actual call you make: when calling request, if you pass a full uri with scheme/host, it works as scope, and if you pass a path, it prepends the base uri.

Seems a lot of work though, and not necessarily on the scope of this PR.