Skip to content

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

Merged
taylorotwell merged 1 commit into
laravel:7.xfrom
codefort-io:7.x
Jul 18, 2020
Merged

[7.x] Add a base exception for Http exceptions#33581
taylorotwell merged 1 commit into
laravel:7.xfrom
codefort-io:7.x

Conversation

@markstaun

@markstaun markstaun commented Jul 18, 2020

Copy link
Copy Markdown
Contributor

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

derekmd commented Jul 18, 2020

Copy link
Copy Markdown
Contributor

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