Как правильно писать catch в php?

Question

[chincharovpc]

Как правильно писать catch, чтобы не повторять код?
метод createDefault генерирует разные ошибки, мне нужно их перехватить и сделать rollback БД и отправить ошибку дальше.

try {
            DB::beginTransaction();
            $project = $this->projectService->createDefault($request);
            DB::commit();
        } catch (AccessDeniedHttpException $e) {
            DB::rollBack();
            throw new AccessDeniedHttpException($e->getMessage());
        } catch (BadRequestHttpException $e) {
            DB::rollBack();
            throw new BadRequestHttpException($e->getMessage());
        } catch (Exception $e) {
            DB::rollBack();
            throw new HttpException(500, $e->getMessage());
        }

Answer 1

[FanatPHP]

Для того чтобы правильно писать catch, надо писать осмысленный код.

А здесь мы наблюдаем очаровательный пример карго-культа.
Узнав, что "толстый контроллер - это плохо, а сервисы - это хорошо", автор небрежным движением руки замел весь мусор под ковёр перенёс весь код из контроллера в "сервис". Ну а что? Контроллер худой, весь код в сервисе!
Тот факт, что сама кривая структура проекта осталась, по сути, той же самой, нас не смущает.
Как и то, что сервис вдруг начал выполнять функции НТТР контроллера и кидать почему-то НТТР исключения. Что с этими исключениями делать в случае, если тот же сервис будет вызван из консольной команды - загадка.

Но самое забавное, что при всём при этом контроллер всё равно пытается выполнять работу модели. Казалось бы, какое отношение интерфейс для обслуживания НТТР запросов имеет к транзакциям в базе данных? А вот поди ж ты!

Чтобы сделать этот код осмысленным, контроллеру всё-таки придется потрудиться, и выполнить какую-то работу самому, а не перекладывать на "сервис". А так же отдать модели то что ей принадлежит.

В общем транзакцию перекинуть в createDefault. причём не напрямую, а ещё ниже - в слой для работы с БД. Стартовать транзакцию до валидации данных - это как бы *не совсем логично*, мягко говоря. И в итоге, как по волшебству, весь этот говнокод исчезнет как страшный сон.
При этом в параметрах передавать не НТТР реквест чохом, а осмысленный набор параметров, вынутый предварительно из реквеста!
В частности, если модель сама проверяет права доступа, то и передавать ид пользователя из авторизации.

При этом модель не должна кидать НТТР исключения. Она должна кидать исключения бизнес-логики. Которые контроллер уже может ловить и транслировать в хттп. Но тут видимо уже сложнее, поскольку это ж ларавель судя по всему.

В любом случае, уж catch (Exception $e)-у тут точно не место

Но это если рассматривать твой конкретный случай.
В общем же случае правильный код написал Илья.

То есть внутри трая операции с БД и коммит.
в кетче роллбэк и перевыброс исключения. Только ловить надо Throwable

try {
  DB::beginTransaction();
  // запись в БД
  // запись в БД
  // запись в БД
  DB::commit();
} catch (\Throwable $e) {
  DB::rollBack();
  throw $e;
}

Comments

[chincharovpc]

Спасибо!
Если контроллер получает несколько десятков параметров, то передавать их все, а не request?
Как лучше генерировать ошибки на уровне Сервисов, чтобы контроллеру превратить их в http ошибки?

[FanatPHP]

Можно передать массив. или DTO. Заполнив их из реквеста.
Зависит от модели. от того, с какими она данными работает.
но главное чтобы разные части приложения ничего не знали друг о друге
в этом весь смысл MVC и прочих красивых слов, которыми ты пытаешься пользоваться.
модель ничего не должна знать про НТТР реквест
контроллер не должен знать про базу данных

Answer 2

[Илья]

Ну я бы наверное так сделал

DB::beginTransaction();

try {
	$project = $this->projectService->createDefault($request);
	DB::commit();
} catch (Exception $e) {
	DB::rollBack();
	throw $e;
}

UPD: Судя по всему это laravel? можно ещё проще тогда:

DB::transaction(function() use ($request) {
	$project = $this->projectService->createDefault($request);
});

Comments

[FanatPHP]

коммит засунь внутрь try

[Илья]

FanatPHP, да, ошибочка.

[FanatPHP]

транзакция в контроллере? вот серьёзно, прям серьёзно?

[Илья]

FanatPHP, эм... это мне вопрос? Где автор это будет писать я не знаю. Да мне и всё равно, если честно, хоть на лбу путь пишет. Я всего лишь на вопрос ответил по заданному куску кода.

[FanatPHP]

Ладно, извиняюсь.
Если рассматривать более общий кейс, то хороший пример

[chincharovpc]

FanatPHP, транзакции в сервис перенести?