[9.x] Allow HTTP client requests with retries to optionally throw RequestExceptions #40076

mattkingshott · 2021-12-16T16:51:38Z

This PR addresses #40055 by adding functionality to the HTTP client so that you can prevent it from throwing a RequestException when retries are configured.

The problem

Presently, and as highlighted in the docs, the HTTP client does not throw exceptions when a request fails. As a result, you can make use of methods like failed on the response e.g.

$response = Http::get('http://example.com');

if ($response->failed())
{
    // Handle the error
}

However, if you configure retries on the HTTP client, it will throw a RequestException if all attempts fail.

$response = Http::retries(3, 1000)->get('http://example.com');

This means:

You cannot configure throw() on the client.
You cannot chain methods e.g. throw()->json().
You cannot use throwIf.
You cannot use the response's error handling methods e.g. $response->failed().

In addition, you have to wrap the request within a try/catch block if you want to handle the error.

The solution

The PR adds a fourth parameter throw to the retries method, which allows you to control whether the HTTP client should throw an exception when the retry limit is reached.

By default, this parameter is set to true to preserve backward compatibility.

Http::retries(3, 1000); // throws a RequestException if all retries fail
Http::retries(3, 1000, null, true); // throws a RequestException if all retries fail
Http::retries(3, 1000, null, false); // does not throw a RequestException if all retries fail

…" (#40054) This reverts commit 608c430.

…nce key from cache in Redis driver (#40039) * [8.x] Fix possible out of memory error when deleting values by reference key from cache in Redis driver * formatting Co-authored-by: Denis Kuzmenchuk <[email protected]> Co-authored-by: Taylor Otwell <[email protected]>

* Add setApplication method to FilesystemManager * Update FilesystemManager.php * Update src/Illuminate/Filesystem/FilesystemManager.php Co-authored-by: Noël Hagestein <[email protected]> * Update FilesystemManager.php Co-authored-by: Taylor Otwell <[email protected]> Co-authored-by: Noël Hagestein <[email protected]>

### Summary This pull request adds a new way to define attribute "accessors / mutators" as they are currently called in the documentation: https://laravel.com/docs/8.x/eloquent-mutators#accessors-and-mutators Currently, accessors and mutators are added to a model by defining `get{Foo}Attribute` and `set{Foo}Attribute` methods on the model. These conventionally named methods are then used when the developers attempts to access the `$model->foo` property on the model. This aspect of the framework has always felt a bit "dated" to me. To be honest, I think it's one of the least elegant parts of the framework that currently exists. First, it requires two methods. Second, the framework does not **typically** prefix methods that retrieve or set data on an object with `get` and `set` - it just hasn't been part of Laravel's style (e.g., `$request->input()` vs. `$request->getInput()`, `$request->ip()` vs. `$request->getIp`, etc. This pull request adds a way to define attribute access / mutation behavior in a single method marked by the `Illuminate\Database\Eloquent\Casts\Attribute` return type. In combination with PHP 8+ "named parameters", this allows developers to define accessor and mutation behavior in a single method with fluent, modern syntax by returning an `Illuminate\Database\Eloquent\Casts\Attribute` instance: ```php /** * Get the user's title. */ protected function title(): Attribute { return new Attribute( get: fn ($value) => strtoupper($value), set: fn ($value) => strtolower($value), ); } ``` ### Accessors / Mutators & Casts As some might have noticed by reading the documentation already, Eloquent attribute "casts" serve a very similar purpose to attribute accessors and mutators. In fact, they essentially serve the same purpose; however, they have two primary benefits over attribute accessors and mutators. First, they are reusable across different attributes and across different models. A developer can assign the same cast to multiple attributes on the same model and even multiple attributes on different models. An attribute accessor / mutator is inherently tied to a single model and attribute. Secondly, as noted in the documentation, cast classes allow developers to hydrate a value object that aggregates multiple properties on the model (e.g. `Address` composed of `address_line_one`, `address_line_two`, etc.), immediately set a property on that value object, and then `save` the model like so: ```php use App\Models\User; $user = User::find(1); $user->address->lineOne = 'Updated Address Value'; $user->save(); ``` The current, multi-method implementation of accessor / mutators currently **does not allow this** and it can not be added to that implementation minor breaking changes. However, this improved implementation of attribute accessing **does support proper value object persistence** in the same way as custom casts - by maintaining a local cache of value object instances: ```php /** * Get the user's address. */ protected function address(): Attribute { return new Attribute( get: fn ($value, $attributes) => new Address( $attributes['address_line_one'], $attributes['address_line_two'], $attributes['city'], ), set: fn ($value) => [ 'address_line_one' => $value->addressLineOne, 'address_line_two' => $value->addressLineTwo, 'city' => $value->city, ], ); } ``` ### Mutation Comparison In addition, as you may have noticed, this implementation of accessors / mutators **does not** require you to manually set the properties in the `$this->attributes` array like a traditional mutator method requires you to. You can simply return the transform value or array of key / value pairs that should be set on the model: "Old", two-method approach: ```php public function setTitleAttribute($value) { $this->attributes['title'] = strtolower($value); } ``` New approach: ```php protected function title(): Attribute { return new Attribute( set: fn ($value) => strtolower($value), ); } ``` ### FAQs **What if I already have a method that has the same name as an attribute?** Your application will not be broken because the method does not have the `Attribute` return type, which did not exist before this pull request. **Will the old, multi-method approach of defining accessors / mutators go away?** No. It will just be replaced in the documentation with this new approach.

driesvints · 2021-12-16T19:26:18Z

src/Illuminate/Http/Client/PendingRequest.php

     * @return $this
     */
-    public function retry(int $times, int $sleep = 0, ?callable $when = null)
+    public function retry(int $times, int $sleep = 0, ?callable $when = null, bool $throw = true)


We can't change the method signature on a stable release unfortunately.

Should I submit this to master then? So for v9

Yeah, if we can't solve it differently then that would be best.

Okay. I’ll submit it for master.

TBlindaruk and others added 20 commits December 14, 2021 22:46

[8.x] update changelog

decb0a7

Merge remote-tracking branch 'upstream/8.x' into 8.x

087232e

[8.x] update changelog

323942a

fix change log (#40052)

a19f2f6

Revert "[8.x] Remove redundant description & localize template (#39928)…

2ccb714

…" (#40054) This reverts commit 608c430.

Add missing method to Collection and LazyCollection

5a53592

Fix code style

09689f2

rename method

3e3cbcf

patch

c67acfd

Update CHANGELOG-8.x.md

588537d

fix arg passing in doesntContain

739d847

Update PendingRequest.php

7d34cb3

Update Factory.php

2909679

Update HttpClientTest.php

282c65c

Update HttpClientTest.php

0c05a4a

Update PendingRequest.php

e534343

Update HttpClientTest.php

fa79437

GrahamCampbell changed the title ~~Allow HTTP client requests with retries to optionally throw RequestExceptions~~ [8.x] Allow HTTP client requests with retries to optionally throw RequestExceptions Dec 16, 2021

driesvints reviewed Dec 16, 2021

View reviewed changes

mattkingshott changed the base branch from 8.x to master December 16, 2021 19:31

mattkingshott changed the title ~~[8.x] Allow HTTP client requests with retries to optionally throw RequestExceptions~~ [9.x] Allow HTTP client requests with retries to optionally throw RequestExceptions Dec 16, 2021

mattkingshott closed this Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[9.x] Allow HTTP client requests with retries to optionally throw RequestExceptions #40076

[9.x] Allow HTTP client requests with retries to optionally throw RequestExceptions #40076

mattkingshott commented Dec 16, 2021

driesvints Dec 16, 2021

mattkingshott Dec 16, 2021 •

edited

Loading

driesvints Dec 16, 2021

mattkingshott Dec 16, 2021

[9.x] Allow HTTP client requests with retries to optionally throw RequestExceptions #40076

[9.x] Allow HTTP client requests with retries to optionally throw RequestExceptions #40076

Conversation

mattkingshott commented Dec 16, 2021

The problem

The solution

driesvints Dec 16, 2021

Choose a reason for hiding this comment

mattkingshott Dec 16, 2021 • edited Loading

Choose a reason for hiding this comment

driesvints Dec 16, 2021

Choose a reason for hiding this comment

mattkingshott Dec 16, 2021

Choose a reason for hiding this comment

mattkingshott Dec 16, 2021 •

edited

Loading