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

[7.x] Add a base exception for Http exceptions #33581

Merged
merged 1 commit into from
Jul 18, 2020

Conversation

markstaun
Copy link
Contributor

@markstaun markstaun commented Jul 18, 2020

I suggest that we add a base exception that the exceptions thrown by the Http client can extend.

By doing that it's easy to catch any errors that will be thrown.

Here is a real-life Stripe portal integration before and after.

Before

try {
    $response = Http::asForm()
        ->retry(3, 500)
        ->timeout(5)
        ->withBasicAuth($secret, '')
        ->post('https://api.stripe.com/v1/billing_portal/sessions', [
            'customer' => $client->stripe_customer_id,
            'return_url' => $returnUrl,
        ])->throw();
} catch (RequestException $exception) {
    throw ClientBillingException::make('Failed to redirect to Stripe portal.', $exception);
} catch (ConnectionException $exception) {
    throw ClientBillingException::make('Failed to redirect to Stripe portal.', $exception);
}

After

try {
    $response = Http::asForm()
        ->retry(3, 500)
        ->timeout(5)
        ->withBasicAuth($secret, '')
        ->post('https://api.stripe.com/v1/billing_portal/sessions', [
            'customer' => $client->stripe_customer_id,
            'return_url' => $returnUrl,
        ])->throw();
} catch (HttpClientException $exception) {
    throw ClientBillingException::make('Failed to redirect to Stripe portal.', $exception);
}

This is a very simple addition which will not be a breaking change, as people can still catch the individual exceptions or the \Exception class.

I'm open to other names for the base exceptions.

I've called it "HttpClientException", but other names could be:

  • ClientExecption
  • HttpException
  • BadResponseException

You could argue that we can just catch the \Exception class, but I don't want to do that in case that any exceptions are thrown by some other logic in the try-catch block. This could e.g. be an exception thrown by $client->stripe_customer_id

@GrahamCampbell GrahamCampbell changed the title Add a base exception for Http exceptions [7.x] Add a base exception for Http exceptions Jul 18, 2020
@taylorotwell taylorotwell merged commit 29ad356 into laravel:7.x Jul 18, 2020
@derekmd
Copy link
Contributor

derekmd commented Jul 18, 2020

FYI for the first example with common catch blocks, PHP 7.1 added catching multiple exception types so the code snippet could be shortened to:

try {
    // ....
} catch (ConnectionException | RequestException $e) {
    throw ClientBillingException::make('Failed to redirect to Stripe portal.', $e);
}

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.

3 participants