0

I am implementing payments for my website using the API of an external service (ie. the service of the payment provider).

Let's say the user clicks 'BUY', and then we go to my controller which says something along the lines of:

public function buyFunction() {

    $result = $this->ExternalService->pay();

    if ($result->success == true) {
        return 'We are happy';
    }
}

I have also created the aforementioned externalService which has the pay() method:

class ExternalService {

    public function pay() {
        response = //Do stuff with Guzzle to call the API to make the payment

        return response;
    }
}

Now, sometimes things go wrong. Let's say the API returns an error - which means that it throws a GuzzleException - how do I handle that?

Ideally, if there is an error, I would like to log it and redirect the user to a page and tell him that something went wrong.

What I've tried

  1. I have tried using a try/catch statement within the pay() function and using abort(500) but this doesn't allow me to redirect to the page I want to.

  2. I have tried using a try/catch statement within the pay() function and using return redirect('/mypage') but this just returns a Redirect object to the controller, which then fails when it tries to call result->success

  3. I have tried using number 2 but also adding a try/catch block to the controller method, but nothing changed.

In the end, I have found two solutions. In both, I use a try/catch block inside the pay() method. Then I either return 0; and check in the controller if (result == 0) or I use abort( redirect('/mypage') ); inside the try/catch block of the pay() method.

What is the right way to handle this? How to use the try/catch blocks?

padawanTony
  • 1,348
  • 2
  • 22
  • 41
  • 1
    I would suggest the `pay()` function should catch the Guzzle exception and throw one of your own (if you don't have your own exception classes, don't do anything.) Then in your controller, you will catch the exception and redirect to an error message. Your controller handles end-user interactions like aborts or redirects, this is not the job of a payment processing class. – miken32 Dec 05 '20 at 00:07
  • Just a note, i would suggest to follow standard class naming using pascal case so ExternalService instead of externalService :) – mrhn Dec 05 '20 at 00:29
  • @miken32 so every function of the controller should have a try/catch block? I feel like I have to use try/catch blocks all the time – padawanTony Dec 06 '20 at 05:23
  • @miken32 Also what would be the advantage of creating my own exception class instead of catching Guzzle's exception? – padawanTony Dec 06 '20 at 07:43
  • 1
    It depends on your use case. You may have multiple exception classes to watch for in the controller, or you may have special actions you want to happen when an exception is thrown. And if you’re writing code for use by third parties it’s neater to throw your own exceptions. – miken32 Dec 06 '20 at 12:06
  • Thanks. Btw, you can throw your custom exception and, instead of using a try/catch block in the controller, handle it in the `render` method of the custom Exception. This is more neat IMO since you don't bloat your controller with try/catch blocks. – padawanTony Dec 06 '20 at 12:33

1 Answers1

1

In my experience, avoid handling exceptions let them pass through and handle them accordingly with try catches. This is the most pragmatic approach. Alternatively you will end up checking result is correct in weird places, eg. if ($result) {...}. Just assume it went good, except if the exception is thrown. Bonus: never do Pokemon catches with Exception $e unless you specifically needs it!

class ExternalService {

    public function pay() {
        try {
            response = $client->get(...);
        } catch (BadResponseException $exception) {
            Log::warning('This should not happen check payment api: ' . $exception->getMessage());
            
            throw new PaymentException('Payment did not go through');
        } 

        return response;
    }
}

Assuming you have your own Exception.

class PaymentException extends HttpException
{
    public function __construct(?\Exception $previous = null)
    {
        parent::__construct(Response::HTTP_BAD_REQUEST, 'Unexpected error processing the payment', $previous);
    }
}

This enables you to handle the flow in a controller, where it would make sense to handle the redirect. Sometimes if the exception is very integral or common to the web app, it can also be handled by the exception handler instead.

class PaymentController {
    public function pay(PaymentService $service) {
        try {
            $payment = $service->buyFunction();
        } catch (PaymentException $exception) {
            return redirect()->route('app.payment.error');
        } 
        
        return view('app.payment.success', compact('payment'));
    }
}
mrhn
  • 17,961
  • 4
  • 27
  • 46
  • Thank you for your answer. My questions: 1) Why should I create my own custom Exception when a specific GuzzleException already exists? 2) Why shouldn't I just catch a generic Exception - ex: `catch (\Exception $e)`? This way I can include more code in my try/catch block. And whichever line of my code breaks, I will catch it. 3) Do I really need to add a try/catch block in every line? I feel like I have to use it a lot. 4) My catch block is big and my controller gets bloated. – padawanTony Dec 06 '20 at 05:19
  • 1. Because your user doesnt care about guzzle failing wrapping it in something you can more easily handle is better, you would be able to use PaymentException in all sort of cases but guzzle exceptions the user would never care about and is in general considered good practice and easier to track in systems like bugsnag sentry etc. Where guzzle exception can be many things. 2. Because if you make an ArrayIndexOutOfBounds error it will also handle that, that can be very hard to debug, if you are specific you secure weird exceptions are not going through. – mrhn Dec 06 '20 at 16:20
  • Thank you. Btw, instead of using a try/catch block in the controller, you can handle it in the render method of the custom Exception. This is more neat IMO since you don't bloat your controller with try/catch blocks. – padawanTony Dec 10 '20 at 00:20
  • Yes you can :) it is up to the case where to handle it, i think what a controller does by design doing some try catch is not bad. But if the error is more general the Handler.php is to be preferred. – mrhn Dec 12 '20 at 12:06