Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Makes the retrieval of Http client transferStats safe #37597

Merged
merged 3 commits into from
Jun 4, 2021
Merged

[8.x] Makes the retrieval of Http client transferStats safe #37597

merged 3 commits into from
Jun 4, 2021

Conversation

lukeraymonddowning
Copy link
Contributor

Hello there!

The Http Client returns some really nice details in the handlerStats method. However, when faking a request inside a test, the transferStats object is null. This causes an error when attempting to call the handlerStats method. This PR updates the handlerStats call by wrapping it in an optional call and returning an empty array by default. This makes consuming code much cleaner by no longer requiring a try/catch statement.

Http::fake(['example.com' => Http::response()]);

// Before
try {
    $stats = Http::get('example.com')->handlerStats();
} catch ($e) {
    $stats = [];
}

// After
$stats = Http::get('example.com')->handlerStats()['some_stat'] ?? null;

There is another option, which is that we return a new Fluent from handlerStats, but that would be a breaking change because previous try/catches would now pass but return null, which could cause undesired side-effects. If that is desirable to you, let me know and I'll write a PR for 9.x that will expand on this one.

Thanks for your time guys :-)

Regards,
Luke

@lukeraymonddowning
Copy link
Contributor Author

I've also added in support for effectiveUri, which was also throwing an error when faking requests. It will now return null when faked.

lukeraymonddowning added a commit to lukeraymonddowning/laravel-ray that referenced this pull request Jun 4, 2021
@taylorotwell taylorotwell merged commit 9448d7e into laravel:8.x Jun 4, 2021
@lukeraymonddowning lukeraymonddowning deleted the handler-stats-safety-net branch June 4, 2021 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants