Laravel Controller-Model Exception Processing a structure with database transactions

Question

As for architecture, which of the two is good practice when throwing exceptions from a model to a controller?

Structure A:

UserController.php

public function updateUserInfo(UserInfoRequest $request, UserModel $userModel) { $isError = false; $message = 'Success'; try { $message = $userModel->updateUserInfo($request->only(['username', 'password'])); } catch (SomeCustomException $e) { $isError = true; $message = $e->getMessage(); } return json_encode([ 'isError' => $isError, 'message' => $message ]); } 

UserModel.php

 public function updateUserInfo($request) { $isError = false; $message = 'Success'; $username = $request['username']; $password = $request['password']; try { $this->connect()->beginTransaction(); $this->connect()->table('users')->where('username', $username)->update(['password' => $password]); $this->connect()->commit(); } catch (\Exception $e) { $this->connect()->rollback(); $isError = true; $message = $e->getMessage(); } return [ 'isError' => $isError, 'message' => $message ]; } 

Structure B:

UserController.php

 public function updateUserInfo(UserInfoRequest $request, UserModel $userModel) { $isError = false; $message = 'Success'; try { $userModel->updateUserInfo($request->only(['username', 'password'])); } catch (SomeCustomException $e) { $isError = true; $message = $e->getMessage(); } catch (QueryException $e) { $isError = true; $message = $e->getMessage(); } return json_encode([ 'isError' => $isError, 'message' => $message ]); } 

UserModel.php

 public function updateUserInfo($request) { $username = $request['username']; $password = $request['password']; try { $this->connect()->beginTransaction(); $this->connect()->table('users')->where('username', $username)->update(['password' => $password]); $this->connect()->commit(); } catch (\Exception $e) { $this->connect()->rollback(); throw new QueryException(); } } 

In Structure A, the model catches any exception, rolls back the transaction, and returns if it has an error or not for the controller. Then the controller returns only what is returned from the model.

In Structure B, the model catches any exception, rolls back the transaction, and throws a QueryException if an exception occurs. The controller then catches the received QueryException from the model, and then returns if it has an error or not.

The reason that Structure B still has a catch is because the model needs to be rolled back. If I were to remove the try-catch on the model here and the controller in order to immediately catch the exception, then the rollback would be handled on the controller, which, I think, clutters the functionality of the controller.

Let me know your thoughts. Thanks!

Answer 1

Why I think the approach from B is better:

• Your model should include only the logical part: this includes communication with the database (transaction and rollback), not formatting the error message that you want to print to the user.

• Keep your model clean: this is the most important part of the MVC framework. If you mess it up, it will be very difficult to find any errors.

• Error handling outsourcing: if you put it in the controller, you have the choice to handle it (maybe you need some special formatted output for this method or you need other functions to call), or you handle it in App\Exceptions\Handler In this case, you can display this error message here and do not need to do this in the controller.

So, if you don’t need special function calls and want to use all the power of Laravel, I would suggest you a C Structure

UserController.php

 public function updateUserInfo(UserInfoRequest $request, UserModel $userModel) { $userModel->updateUserInfo($request->only(['username', 'password'])); return response()->json(['message' => 'updated user.']); } 

UserModel.php

 public function updateUserInfo($request) { $username = $request['username']; $password = $request['password']; try { $this->connect()->beginTransaction(); $this->connect()->table('users')->where('username', $username)->update(['password' => $password]); $this->connect()->commit(); } catch (\Exception $e) { $this->connect()->rollback(); throw new QueryException(); } } 

App \ Exceptions \ Handler

 public function render($request, Exception $exception) { //catch everything what you want if ($exception instanceof CustomException) { return response()->json([ 'message' => $exception->getMessage() ], 422); } return parent::render($request, $exception); } 

You have a clear separation of database material (Model), presentation material (Controller), and error handling (handler). The C structure allows you to reuse error handling in other functions where you have the same situation in another controller function.

This is my opinion, but I am open to discuss any scenario in which you think that this approach is not the best solution.

Answer 2

I would rather keep the controllers and any other part of the system that interacts with the models, as agnostic as possible about the internal workings of the model. So, for example, I will try to not know about QueryException outside the model and instead consider it as a simple PHP object when possible.

I would also avoid the custom JSON response structure and use HTTP statuses . If that makes sense, perhaps the user information update route returns an updated resource, or maybe 200 OK .

 // UserModel.php public function updateUserInfo($request) { $username = $request['username']; $password = $request['password']; try { $this->connect()->beginTransaction(); $this->connect()->table('users')->where('username', $username)->update(['password' => $password]); $this->connect()->commit(); return $this->connect()->table('users')->where('username', $username)->first(); // or just return true; } catch (\Exception $e) { $this->connect()->rollback(); return false; } } // UserController.php public function updateUserInfo(UserInfoRequest $request, UserModel $userModel) { $updated = $userModel->updateUserInfo($request->only(['username', 'password'])); if ($updated) { return response($updated); // HTTP 200 response. Returns JSON of updated user. // Alternatively, // return response(''); // (200 OK response, no content) } else { return response('optional message', 422); // 422 or any other status code that makes more sense in the situation. } 

---

(Completely off topic, I think this is an example, but just in case, a reminder does not store passwords with clear text.)

Answer 3

First of all , for your example, you do not even need to use a transaction. You execute only one request. so why do you roll back? Which request do you want to cancel? A transaction should be used when you need a set of changes that need to be fully processed in order to consider the operation complete and valid. If the first one is successful, but any of the following has some kind of error, you can roll back everything as if nothing had been done.

Secondly, let's move on to good practice or best practice. Laravel offers a thin controller and a thick model. Thus, all your business logic should be in the model or even better in the repository. The controller will act as a broker. It will collect data from the repository or model and transmit it for viewing.

Alternatively , laravel provides a convenient and convenient way to organize your codes. You can use Event and Observers to work in parallel in your model.

Best practice depends on user knowledge and experience. So who knows, the best answer to your question is yet to come.

Answer 4

I don’t understand why you didn’t watch Jeffrey’s lesson, but you don’t need a try / catch section to update the user. you use the controller method:

 public function update(UpdateUserRequest $request, User $user) : JsonResponse { return response()->json($user->update($request->all())) } 

you are requesting a rule method:

 public function rules(): array { return [ 'username' => 'required|string', 'password' => 'required|min:6|confirmed', ]; } 

And the visualization method of the exception handler:

 public function render($request, Exception $exception) { if ($request->ajax() || $request->wantsJson()) { $exception = $this->prepareException($exception); if ($exception instanceof \Illuminate\Http\Exception\HttpResponseException) { return $exception->getResponse(); } elseif ($exception instanceof \Illuminate\Auth\AuthenticationException) { return $this->unauthenticated($request, $exception); } elseif ($exception instanceof \Illuminate\Validation\ValidationException) { return $this->convertValidationExceptionToResponse($exception, $request); } // we prepare custom response for other situation such as modelnotfound $response = []; $response['error'] = $exception->getMessage(); if (config('app.debug')) { $response['trace'] = $exception->getTrace(); $response['code'] = $exception->getCode(); } // we look for assigned status code if there isn't we assign 500 $statusCode = method_exists($exception, 'getStatusCode') ? $exception->getStatusCode() : 500; return response()->json($response, $statusCode); } return parent::render($request, $exception); } 

Now, if you have an Exception, Laravel gives you a status code in Json! = 200, otherwise give a success result!