Как знают многие разработчики, обработка ошибочных ситуаций - архиважная составляющая любого полноценного кода, претендующего на использование в реальной жизни.
И как следствие, писать обработку ошибок - прямая обязанность любого ответственного программиста. Некоторые считают, что если вы не предусмотрели все варианты поведения - вы не программист, и диплом вам не дадут (если речь о студентах).
Но с другой стороны, обрабатывать ошибки всегда лениво и напряжно. Поэтому существует много инструментов для облегчения этой задачи. Стандартный механизм обработки ошибок в Java - Exceptions. Я не буду сейчас расписывать скучные описания, как бы отвечая на скучные вопросы со многих собеседований. Скажу лишь, что Исключение - это на мой взгляд, неправильный перевод понятия Exception. Я считаю, что исключение, то есть исключительная ситуация - это про другого представителя летающих монстров, java.lang.Error.
А Exception - это не исключение, это часть правила. Это всегда один из возможных исходов. Я бы перевёл этот класс не иначе как Отклонение (Отступление, Отвод). В смысле отклонение от прямого курса. Потому как перехватив это отклонение, можно курс выправить и продолжить работу. А Исключение - это для безответственных разработчиков.
Так вот. Давайте теперь перехватывать эти отклонения. Как можно это сделать?
В современной разработке существует тенденция наследовать все Exception от непроверяемых исключений. Это позволяет не заботиться о написании кода обработки ошибок. Я считаю этот подход расхлябанным и безответственным. И сам всегда пропагандирую использование явно заданных и объявленных отклонений с жёсткой обязанностью их обработать, т.е. настаиваю на использовании только проверяемых исключений/отклонений в любом бизнес-коде.
Это вносит неудобства, да. Надо их везде ловить. Как упростить задачу? Обычно советуют тупо оборачивать исключение в java.lang.RuntimeException. Но такой подход чреват тяжёлыми последствиями, когда приложение выходит в релиз и на бой. Я как человек, имеющий ещё и плюсовый бэкграунд, прекрасно знаю, что некоторые ситуации невозможно предугадать даже очень внимательно вглядываясь в код. А потом искать их причины днями, неделями, иногда месяцами. Потому что кто-то в самом начале, в месте возникновения ошибки не объявил о возможности их возникновения.
Ну да ладно. Не будем о грустном. Лучше попробуем сделать нашу жизнь проще, написав утилитку, которая с одной стороны, позволяет обрабатывать ошибки. А с другой стороны не привносит глобальных неудобств и не захламляет код бесконечными (часто вложенными друг в друга) блоками try-catch-finally.
Что предлагается?
Во-первых, на каждом уровне-слое приложения должна быть своя логика по контролю ошибок, специфичная для данного слоя. На каждом таком слое есть своё семейство Исключений, желательно проверяемых.
Исключением из правила (простите за каламбур) могут будут только специализированные фреймворки, и то не всегда. Они могут игнорировать ошибки, которые от них не зависят. Но лучше делать это через туннелирование ошибок, то есть обернуть на влёте - развернуть на вылете.
Во-вторых, надо бы упростить синтаксис обработки ошибок. Тут я поймал себя на мысли, что необходимость в этом возникает, так как мэйнстрим постепенно, медленно, но всё-таки верно потихоньку уползает от практик процедурного программирования. А блоки try-catch, по-моему, являются наследием именно этой парадигмы. И ООП , и тем более функциональщина и даже стримы Java-8 как-то плохо сочетаются с этими блоками, фигурными скобками.
Вот я и решил сделать утилитку, которая бы помогла реализовать вышеназванный подход. Предлагаю вашему вниманию её первую версию.
Здесь я попробовал сразу около трёх подходов реализовать.
Первый - просто статичная обёртка над небезопасным кодом, которая напечатает в лог и в случае чего прокинет ошибку своего местного разлива. Это метод errorToWarn.
Второй имел целью дальнейшее снижение шаблонного кода: вынесение инициализации логгеров в одно отдельное место. Для работы нужна только одна статическая функция err2Warn.
И затем я решил пойти ещё дальше и сделать описание ошибок в удобном формате, одновременно предлагая ленивую логику и флюент (как по-русски это?) интерфейс. Это вылилось в вызов, который уже возвращает своеобразный билдер, который затем можно настраивать по типу jmock или spring security api. Выглядеть это стало примерно как табличка состояний, примерно вот так:
К слову первый подход вылился во что-то наподобие
Надеюсь мои лирические рассуждения покажутся вам полезными. Возможно вам понравится использованный мной подход и вы захотите развить утилиту советом / личным вкладом.
Код утилиты: ErrorWrapper.java
По следам обсуждения в комментариях, ссылка на самый авторитетный источник, где все бессмысленные споры прерываются ясным и чётким разъяснением: https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
shalomman
Очень спорные утверждения.
У меня вот какие аргументы против:
mahairod Автор
К слову, на скрине показан случай, когда игнорирование ошибки делает код не просто бесполезным, но и опасным.
michael_v89
Ну вот вызываю я
IStorage::save(), и чего, какие ошибки ловить? Дисковые, сетевые, ошибки прожига CD?mahairod Автор
Если ваш сторадж умеет воспринимать ошибки реализации, а он обязан, то можно рассчитывать на некие обобщенные ошибки сохранения.
Если вы на это не можете полагаться, выкиньте этот сторадж и возьмите другой более правильный тулкит.
michael_v89
Например?
mahairod Автор
Для приведенных вами случаев:
НашСкладНедоступенОшибка
СохранениеПрервалосьОшибка
Более общее — ОшибкаСохраненияНаСкладе.
michael_v89
Ну так чтобы сделать что-то кроме "упасть и записать в лог", надо знать причину, почему он недоступен, почему сохранение прервалось. А такие абстрактные ошибки ничем не отличаются от RuntimeException. А ConnectionError к чему относить, это склад недоступен или сохранение прервалось? А если мы хотим сохранить, и склад недоступен, это ведь тоже означает, что сохранение прервалось, почему это разные исключения?
Типизированные исключения для того и придумали, чтобы можно было отлавливать конкретные известные ошибки, тогда известно и как их исправить. Ошибку соединения по сети и ошибку прожига CD обрабатывать надо по-разному.
oxidmod
Так то это тип ошибки, а в деталях будет конкретное сообщение для каждого конкретного хранилища, будь-то база данных или что-то другое
mahairod Автор
Если у вас склад недоступен — то вам в общем-то не важно, что послужило этому причиной: уборщица кабель отключила или диск сбоит. Вы поставлены перед фактом: сейчас сохранить не могу, нет доступа к хранилищу. И дальше уже ваша ответственность на основе этих сведений принимать решение.
Как выше было замечено, в яве обычно сохраняют исходную причину сбоя, так что при большом желании можно посмотреть детали. Я к примеру, всегда разворачиваю InvocationTargetException, потому что само исключение не имеет никакого смысла.
ConnectionError обрабатывается в зависимости от контекста. Для абстракции склада нас не интересует какого типа ошибка произошла в реализации (уборщицу будут наказывать люди после просмотра логов, а не ваш сервис), нас интересует, что определённого типа операции нам временно/постоянно недоступны.
Склад недоступен — это когда даже начать операцию нельзя, например сервис вообще не отвечает. Операция прервана — значит взаимодействие есть, но состояние склада вызывает вопросы: можно ли к нему обращаться в следующий раз.
Как по разному? У вас склад не работает и так, и эдак. То, что потом, посмотрев логи, вы позвоните админу/уборщице с приказом восстановить сеть или тому же админу поручите заменить/прочистить диски в устройстве — это дело дестятое, организационное и к вашему сервису уже не имеет отношения, никак в данную секунду ему не поможет.
michael_v89
Я об этом и говорю — на основе этих сведений можно только упать с записью в лог. А чтобы это сделать, исключения в сигнатуре функции не нужны.
Ну так вот для сети это одни операции, а для CD диска другие. И обработка для них нужна другая. Ну если мы хотим их как-то обработать, мы же за этим возможные типы в сигнатуре отслеживаем? Для сети можно то же самое действие повторить, а для ошибки записи "сбойный сектор" надо в другой сектор записать. Ок, сектор не очень подходящий пример, в другую ноду кластера например записать если текущая недоступна.
Ну так я ее уже начал вызовом функции save().
Ну так и какой тогда смысл в исключениях в сигнатуре, если мы все равно ничего не можем с ними сделать?
mahairod Автор
Зачем же падать? Может у вас это сохранение было опционально, или имеется другой резервный склад, чуть поодаль.
К тому же если просто упасть, то вы не освободите коннект, ваш поток завершится, сокет будет висеть. Если поток был из пула и произойдёт ещё ~64к запусков потока и попыток подключиться — ваше приложение вообще перестанет отвечать, если к тому времени оно ещё будет живо. Так что падать с лапками кверху — это самое глупое, что можно сделать.
По идее это ваш склад должен делать сам.
Исключения в сигнатуре — чтобы явно видеть, на что рассчитывать, когда зовёшь метод. Из моей практики, не называя имён: сервис не объявил вообще никаких исключений, ни проверяемых, ни не-. В документации по этому поводу тишина. Сервис корный, очень системный. В итоге вызвав этот метод, я получаю ошибки только после тяжёлого развёртывания на стенде. Какого х* спрашивается?
Причем из логики (кода) сервиса становится понятно, что кинуть рантайм-эксепшн, если в системе не настроены кое-какие параметры — в порядке вещей. И эти параметры не в проперти-файлах, а нетривиально задаются отдельно. У меня вопрос, я методом тыка должен об этом узнавать?
mahairod Автор
Да, ещё момент: игнорирование ошибок в методе — это пренебрежение к пользователю вашего метода. Вы экономите своё время за счёт времени вашего контрагента. Он точно рад не будет.
michael_v89
Так и должно быть. На стенде нет чего-то, что нужно приложению для правильной работы — приложение не должно продолжать работу.
Ну вот в этом как раз и проблема, что исключения кидаются в неисключительных ситуациях, когда работу можно продолжать. А вот если бы возврат был, ничего бы не упало.
Так не надо их игнорировать. Вон с Either говорят, компилятор все проверяет.
mahairod Автор
Имелось в виду игнорирование объявления ошибок в сигнатуре.
michael_v89
Игнорироваться должны такие ситуации, которые аналогичны OutOfMemoryException. Вы же не объявляете его в каждой функции. Он происходит редко, и сделать с ним ничего нельзя.
mahairod Автор
Ну во-первых, OutOfMemoryError а не эксепшн. Это ошибка ДЖВМ, я уже несколько раз спорил о том, можно ли вообще что-то делать в ДЖВМ, когда происходят именно Эрроры. На мой взгляд, в случае с OutOfMemoryError продолжать категорически нельзя.
Впрочем, его-то как раз ловить не надо, пусть падает сам.
Но если вдруг случайно поймали — в томкате например, надо звать java.lang.System#exit прям сразу после лога.
SimSonic
Ну если, например, ваш код вызвал new long[Integer.MAX_VALUE], то именно тут можно и поймать OOM, и как-то обработать (запросить меньше?), и пойти выполняться дальше.
Throwable
Вызывающий код абсолютно не обязан обрабатывать ошибки вызываемого кода, а может делегировать это обработчику более высокого уровня. Пример из Java:
Здесь метод writeInt() вызывает OutputStream.write() аж 4 раза, при этом ни нет одного обработчика! Таким же образом любой код вправе сохранить "оптимистичную" модель программирования, не заботясь о завершении каждой "небезопасной" операции, и не замусоривая код обработчиками и логикой, которая не относится к этому методу. Здесь обязуют декларировать checked IOException, однако если бы тут был RuntimeException декларация отсутствовала бы. И такой код вполне себе имеет право на существование.
Это ошибочное мнение.
Во-первых, как я уже сказал, вызывающий метод отнюдь не несет ответственность за обработку возможного исключения, если это въявную не предусмотрено его бизнес-задачей.
Во-вторых, вы никогда не сможете обработать все ошибочные исходы, ибо их множество в общем случае вычислимо, но никогда не определено ни одним контрактом. Я имею ввиду всевозможные RuntimeException, которые могут возникнуть в вызываемом коде, особенно если имеете дело со сторонней библиотекой/фреймворком, где под капотом подмешано большое количество различных технологий. И это множество зависит от многих факторов, многие из которых определяются не кодом, а runtime-конфигурацией системы: для одного и того же интерфейса может быть куча разных подключаемых провайдеров, код может работать в транзакции, вызывать remote-методы, делать RPC, стучаться в базу — и везде есть чему вальнуться. Поскольку конфигурация динамическая, на этапе проектирования нет возможности определить все возможные варианты сбоев.
В-третьих, сильно завязывать бизнес логику на исключения — это заведомо плохой подход. Exception — это именно исключительная ситуация, которая говорит о невозможности выполнения операции и требует раннего завершения всей цепочки вызовов. Если аварийное завершение — это часть вашего контракта и предусмотрена бизнес логикой, то и возвращайте его как обычное значение. Например вместо NotFoundException или NullPonterException возвращайте обычный Optional или на худой конец null.
mahairod Автор
writeInt(int v)не очень удачный пример, потому что здесь однотипные операции с одной и той же ошибкой могут завершиться. И прокидывает наверх потому что это чистый агрегат/делегатор. Здесь нет никакой особой логики внутри.RuntimeException потому так и называется, что он не предназначен для обработки бизнес-логикой. Это теперь уже мы имеем корявое наследие говно-кода в виде разных его обёрток, которые приходится учитывать при обработке ошибок. Исходно для бизнес-ошибок RuntimeException не предназначен. RuntimeException — это ошибка в программе (например NPE), когда надо просто падать.
Если в сигнатуре метода прописаны основные ошибки, которые он генерит, то задача перебора становится реальной. Вы можете посмотреть на аналогию, бывшую в C++: если вы заявляете, что метод генерит только предопределённые исключения, то при попытке выкинуть что-то иное происходит системный сбой, обычно с немедленным завершением работы. И это логично, потому что все нижележащие ошибки метод должен был обработать сам. А то, что не может — объявляет явным образом в своей сигнатуре, то бишь контракте. То же самое происходит по логике вещей в Яве: вы явно выкидываете проверяемые исключения, а непроверяемые — это всегда должен быть только результат системного сбоя / ошибки программирования. Использование RuntimeException, означающего сбой, для целей бизнес-логики — это дизайнерский баг (или баг в мозгах).
Итак, ошибки, ожидаемые от сервиса, известны, они объявлены. Тогда подключая соответсвующую либу/фреймворк, вы обязаны эти ошибки обработать. В свою очередь, наружу вы выставляете только то, что надо знать и уметь вашему клиенту, в том числе типы ошибок. И они не должны вообще никак повторять внутренние ошибки внешних либ. Иногда он даже не сможет определить тип этих ошибок, потому что ему соответствующие классы недоступны. И они не должны быть доступны. Потому что это не его дело знать, что произошло внутри, об этом должен был позаботиться непосредственный вызывающий — вы.
Подключая провайдера, вы должны учитывать его специфику, в т.ч. контракт по ошибкам и если он не совпадает с вашим (для общего случая именно так) — то вы делаете маппинг его ошибок на свои. И тут, конечно уже надо включать голову, потому что вашему клиенту не интересны ваши взаимоотношения с вашим субподрядчиком.
Throwable
Извините, но ваши рассуждения слишком наивны и идеалистичны. Что делать в случае, если ваш контракт изначально не определял возможного исключения, но при при смене провайдера это стало нужно? Например есть интерфейс с методом, который всю жизнь вызывался локально, но затем его реализовали через RPC? Ни клиента ни контракт уже не поменять.
В подавляющем большинстве этого более чем достаточно. Как правило в коде абсолютно наплевать где зафейлилась операция и почему — достаточно вывести ее корректно в лог, откатить транзакцию и вернуть ошибку клиенту.
Ну да ну да. Всевозможные RuntimeException-ы особенно объявлены...
NumberFormatException при Integer.parseInt, когда пользователь ввел строку вместо числа — это ошибка программирования или системный сбой? В самой Java API использование checked/unchecked много где неконсистентно. Почему парсинг URL выдает checked MalFormedURLException, а парсинг числа unchecked NumberFormatException?
mahairod Автор
Ага, классический пример. Если надо расширить контракт — расширяйте, в смысле берите новый расширенный контракт и реализуйте. Потому что от вашей новой реализации на старом контракте никто не хочет неожиданностей. Они не заявлены явно, не надо их создавать неявно. Выйдут новые версии клиентов нового контракта — тогда всё будет ок. А ломать логику через коленку не надо.
Обычно приводят список, который вдруг стал работать по сети. Но, во-первых, это как раз нарушение S в SOLID — вообще то грубейшая ошибка. Да ещё строить на этом нарушении какие-то доказательства, хуже не придумаешь.
Если же прям сильно хочется именно старый контракт — то будьте добры соблюдайте его. Он никаких райнтаймов не кидал. А если будет кидать, то это сугубо ваша вина, потому что вы корявый программист, это вы нарушаете SOLID. Поэтому в наказание вам — закопать вашу реализацию, чтоб не было дурного примера.
Ява с точки зрения API хороша, но далеко не идеальна. Меня, например, удивляет, почему разработав сносную библиотеку коллекций, сановцы не продумали создание отдельных интерфейсов для немутабельных версий. Зачем делать эти грязные хаки в виде выброса UnsupportedOperationException в модифицирующих методах списков, множеств и т.п? Нельзя было для такого случая просто не применять эти методы, т.е. создать отдельные ридонли-интерфейсы?
Throwable
По поводу коллекций — так исторически сложилось, а исправить уже нет возможности. В некоторых API immutable collection заменяет Enumeration, но особо не прижилось.
А теперь по поводу checked exceptions вопрос, вы реально топите за то, чтобы все exceptions были checked и пришлось бы писать подобный код?
Это полный контракт данной операции, разве что отсутствует OutOfMemoryError и StackOverflowError.
Боюсь, с таким ригидным подходом до сих пор бы не существовало бы ни Spring, ни JavaEE, ни других фреймворков и технологий в экосистеме. Более слабая и "оптимистичная" парадигма наиболее продуктивна — контракт никогда не отражает всех нюансов, поэтому нужно быть готовым ко всему, но особо на этом не зацикливаться.
mahairod Автор
Не сказать, чтобы я прям топил, но в своих проектах применяю, обычно это не так страшно смотрится почему-то, один-два типа хватает слихвой. Если нет — то наибольший общий знаменатель вполне подходит.
Ваш пример гипертрофирован. Вполне достаточно объявить что-то типа CalculateAmountException — общий предок CannotCalculateAmount и CouldNotCollectAmountPct. Хотя даже эти случаи у вас надуманны.
Если происходит IndexOutOfBoundException, DivisionByZeroException, то их очевидно здесь ловить не надо. Это ошибка программирования, если они происходят, а значит продолжать в принципе небезопасно. mybean.getId() должен быть проверен на нуль ещё до какого-либо использования: mybean.getId().getClass()
Из UnsupportedOperationException | ClassCastException | NullPointerException |
IllegalArgumentException может произойти только NullPointerException, но его проверили выше.
В общем здесь максимум одно проверяемое исключение кидается. Потенциально — вообще без.
Насчёт JavaEE — спорно, они как раз сначала долго всё согласуют, но надо сказать, в итоге у них получается хорошо в последнее время.
А в Spring я вот прям сейчас борюсь с последствиями излишней гибкости.
Кстати, по поводу сервиса и его переезда на RPC: сделайте корректную обработку ошибок, если коннект валится — то всё, это как нуллпойнтер в локальной версии. То есть сервис неработоспособен. В целом конечно всё от ситуации зависит, но в изначальной логике всегда должен быть какой-то канал сообщения ошибок. Иначе она не расширяема по дизайну.
Throwable
Мой пример именно из серии, если бы язык требовал строгое соблюдение контракта, а все исключения были бы проверяемыми. Контракт с List.get() и a/b может кидать исключения, соответственно, следуя вашей идее, вы обязаны его перехватить, даже если по логике он никогда не возникает (List-то об этом не знает!). И в этом случае кинуть какой-нибудь WTFException, который тоже должен являться частью контракта. Ну либо определить IndexOutOfBoundException как возможную ситуацию в контракте метода, выставляя наружу детали реализации. Более того, любой доступ к полям объекта может выдать NPE, поэтому и их тоже по идее нужно заворачивать в exception или пользоваться Optional вместо значения. И это мы еще не коснулись вычислимости типов, где у Java не такие большие возможности, и при каждом кастинге ставить catch ClassCastException. В итоге это все закончится паранойей и шизофренией, которая совершенно не нужна для реальных бизнес задач. Ваш контракт будет нагружен кучей ненужного дерьма, которое не имеет отношения к решаемой задаче и никогда никем не будет испльзоваться.
Так и сделано. Кидается специальное RuntimeException, которое перехватывается каким-то там верхним обработчиком. А бизнес коду абсолютно наплевать — это именно исключительная ситуация, которая не должна происходить в работающей системе. Бизнес код не знает что делать в этом случае и поэтому не должен ставить никаких обработчиков на этот счет. Поэтому и нагружать контракт различными деталями реализации не есть хорошо.
mahairod Автор
Слушайте, я нигде не говорил, что системные ошибки надо объявлять. Напротив, их как раз трогать не надо. Если у вас деление на ноль, то это код некорректный, и его нужно срочно остановить и исправить. При этом Список нигде не кидает проверяемые исключения, т.к. у него нет бизнес-логики, независимой от вызова. Если вызов некорректный — ошибка в коде.
Я настаиваю на объявлении именно бизнес ошибок, если они могут быть, конечно. При этом предпочтительным вариантом являются именно проверяемые исключения, т.к. непровереямые означают баги.
mahairod Автор
Использование RuntimeException в вашем случае — исключительно вынужденная мера, применяемая только для совместимости. Разработка нового контракта, и тем более новой системы — не требует использования костылей, и возводить такой подход в правило нельзя. Костыли вам требуются, потому что изначальный сервис был криво спроектирован.
И ещё раз: при разработке нового функционала объявлять RuntimeException, который означает баг, нет смысла. И выкидывать несуществующий баг, даже необъявленый — грубейшая ошибка дизайна.
0xd34df00d
Я такой по смыслу код пишу в хаскеле, и полет нормальный. Это просто у джавы слишком слабая система типов, и приходится ручками в каждом месте все эти лишние try/catch писать.
phtaran
а как было бы с сильной системой типов?
0xd34df00d
Когда с утра сейчас переписывал на таком языке, заметил, что там две из трёх проверок не имеют никакого смысла, и пример что-то сдулся. Смысл в том, что вы можете написать сигнатуру как
а в реализации использовать что-то вроде
где
injectбудет выполнять все нужные преобразоавния типов.mahairod Автор
Да, насчёт клиентского кода или вызывающего, надо прояснить.
Здесь я не имею в виду непосредственно предшествующий метод. Это может быть и весь колл-стек, и даже другой стек в случае, когда таска запущена асинхронно и запулена в текущий поток. Обработку ошибок в итоге должен сделать кто-то из всей вызывающей структуры.