Исключение — твой друг

    image
    В середине девяностых, когда я переходил с программирования под DOS на Windows, мой наставник познакомил меня с механизмом исключений.
    С тех пор в моём сознании укоренилось мнение: программа, падающая с исключением — плохая программа. Все исключения надо обрабатывать и завершать работу приложения в случае нештатной ситуации самостоятельно.
    И это вполне актуально для обычного приложения под Windows. Ведь в случае падения приложения пользователь получает невнятное сообщение об ошибке и, как результат, негативное восприятие нестабильно работающего приложения.
    Моё мнение начало меняться после знакомства с инструментами автоматической обработки исключений (таких как EurekaLog и аналогов).
    И окончательно поменялось после знакомства с системой отчетов Google Play.
    Этот пост — крик души против тех тысяч примеров, которые учат нас пихать в свой код необдуманные проверки.

    Поводом для написания данного поста стал диалог с товарищем, который просил совета по реализации взаимодействия NDK с Java-кодом. Мне были показаны исходные коды, написанные на основе этого мануала.
    Код, вызывающий вопросы:
        jmethodID method = env->GetStaticMethodID(interfaceClass, "callBack", "(Ljava/lang/String;)V");
        if(!method) {
            LOGE("callback_handler: failed to get method ID");
            return;
        }
        env->CallStaticVoidMethod(interfaceClass, method, js);
    

    Это классический пример кода, который выглядит правильно, но на самом деле только создает новые проблемы.
    Проверка здесь нужна только в одном случае — если метод callBack не обязан существовать и может отсутствовать в штатном режиме приложения.
    Проверки нужны когда мы проверяем результат работы сетевых модулей или когда мы работаем с внешними файлами и т.п.
    То есть, когда ошибка может возникать по вине внешних факторов. В этом случае проверка на ошибки — наша прямая обязанность.
    Ситуация полностью меняется, когда ошибка генерируется нашим кодом и не зависит от внешних факторов.
    Если callBack описан нами и должен присутствовать в коде, проверка не нужна.

    Проверка лишь замаскирует ошибку.

    Очень важно понимать, что маскировка ошибок — это плохо. Гораздо хуже, чем падение приложения с исключением.
    Вы можете сказать, что сообщение выводится в лог и всплывает во время тестирования.
    Ну, отлично, если всплывает. А если не всплывёт? Если пользователю уйдёт неработающий код, который будет просто молча игнорировать вызов callBack?
    Ваши пользователи будут работать с программой, которая не выполняет задуманный функционал. Хорошо, если это какая-нибудь мелочь, не влияющая серьёзно на работу приложения. Но что, если там скрывается важная часть кода, из-за которого все результаты работы пользователя оказываются некорректными?
    Уберите проверку и первый пользователь, столкнувшийся с вызовом callBack, получит падение приложение. А Вы — отчёт об ошибке с заполненным call stack. Как минимум, это обратит Ваше внимание на проблему. Да, пользователь будет недоволен. Но Вы уже через пять часов зальёте на Google Play новую версию и тысячи пользователей даже не узнают об этой проблеме.
    Это гораздо лучше, чем огромная клиентская база, работающая с приложением с миной замедленного действия.

    Проверки на ошибки — это важная часть работы приложения. Ошибки самого разного рода возникают постоянно. Не бывает ни одного запуска сколь-либо сложного приложения, который бы произошел без возникновения нескольких ошибок. Но надо отделять штатные ошибки от нештатных. Штатные ошибки — те ошибки, которые допустимы и влияение которых предсказуемо. Эти ошибки нужно обрабатывать внутри приложения и менять логику работы в зависимости от ситуации. Нештатные ошибки ни в коем случае нельзя обрабатывать стандартными проверками. Нельзя просто взять и засунуть блок кода в if c проверкой на NULL. Вы всегда должны чётко понимать, почему объект стал NULL и чем это грозит в работе вашего приложения.

    Так что же делать с проверками?

    Можно не делать ничего. Просто убрать проверку. В приведённом примере мы точно знаем, что в случае отсутсвия метода будет вызвано исключение на следующем шаге.
    Этот вариант приемлем, но не очень хорош, потому что вызов исключения находится за пределами нашего кода и мы не можем гарантировать, что исключение будет вызвано.
    Поэтому, в идеальном случае, проверка нужна, но не с записью ошибки в лог, а с возбуждением исключения, чтобы гарантировать остановку выполнения кода и отправку отчёта об ошибке.

    Конечно, я не открыл Америку для профессионалов, которые на собственном опыте знают, насколько вредна маскировка ошибок.
    Этот текст нацелен на новичков, которые изучают программирование по примерам из интернета… А примеры, тем временем, пестрят бессмысленными и беспощадными проверками…
    Share post

    Comments 25

      0
      Это все вопрос философии. Сами фреймвоки на Android подталкивают разработчика к работе через исключения.

      На тему того, что не надо ловить исключения — можно поспорить. Не до конца понятно, что мешает отправить себе стек и сообщение о критической ситуации не роняя приложение. И действительно ли так много ситуаций, когда нельзя позволить продолжить пользоваться приложением?
        0
        Надо ловить исключения.
        Надо продолжать работу, если исключение не критичное.

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


          Разные типы исключений требуют разной реакции. Например, ArgumentException не нужно ни глотать, ни пробрасывать — нужно менять вызывающий код.

          image
          +2
          Если полетело исключение, которого вы не ожидали — вы ничего не можете сказать о текущем состоянии процесса. Какие объекты вообще валидны? Что с ними можно сделать?

          В этой ситуации, пытаться «выкарабкаться» — это потенциальное открывание кучи векторов для атаки вашего кода. Любой баг — это потенциальная дыра в безопасности приложения. Попытка работать с багом — это еще большая дыра.
          +3
          Пользуюсь следующим switch'ем:
          Вариант 1. Метод всегда при любых условиях должен возвращать корректное значение. Проверка не нужна. Assert тоже не нужен.
          Вариант 2. Метод может вернуть некорректное значение, но логика приложения построена таким образом, что этой ситуации в принципе не должно происходить. Сталю Assert — программа упадёт сразу после нарушения инварианта.
          Вариант 3. Метод может вернуть некорректное значение при одном из допустимых вариантов использования. Нужно обработать этот случай.

          Дополнительно: метод (или некоторый кусок кода) уже обработан по одному из вышеперечисленных сценариев, но я знаю, что этот код в принципе может упасть (ну вот кажется, что такое возможно). Однако я могу локализовать проблему вида «этот кусок кода полностью не сработал» и обработать её именно в таком представлении. В этом случае я ставлю на весь этот кусок дополнительный try-catch (Throwable) с логированием исключения и обработкой ситуации «не понятно, что случилось, но этот кусок кода не сработал». Соответственно, если мы имеем дело с мобильным приложением, то в этом месте также нужно реализовать отправку на сервер разработчика этого события.
            +1
            Действую примерно таким же способом.

            Елинственное, не понятно, почему вы шлете репорт только на мобильной версии.
            На десктопе это даже актуальнее, потому что на декстопе отсутствуют встроенные инстурменты оповещения об ошибке.
              0
              Да я как-то больше сервер-сайд, а там логи всегда доступны. Про десктоп согласен.
            +1
            Статья слишком сумбурная, очень много воды.
            Очевидно, что нужно перехватывать ошибки там, где это нужно.
            Если метод не должен отвечать за определенное поведение — не нужно туда его прибивать.

            Очевидно, что передавая строку, которая хранит имя метода, который вы из C++ хотите вызвать, вы полагаете, что он и вызовется.
            Но такой код может НЕ сработать только в одном случае — неверные исходные данные (скорее всего ошибка даже одной буквой, даже капсом в переданной строке) и в продакшн никогда не попадет, именно по этой причине мы просто говорим в логах об ошибке и выходим (повторюсь, такой результат будет только при разработке приложения).
            Для продакшна можно вообще убрать все эти if-return и ничего не изменится.

            Полный код, который вы критикуете
            Тут явно делается попытка вызова по переданному указателю на строку. Любой if-return можно заменить на «Девелопер накосячил и такого метода мы не находим, поправьте имя и запустите еще раз».

            static void callback_handler(char *s) {
                int status;
                JNIEnv *env;
                bool isAttached = false;
               
                status = gJavaVM->GetEnv((void **) &env, JNI_VERSION_1_4);
                if(status < 0) {
                    LOGE("callback_handler: failed to get JNI environment, "
                         "assuming native thread");
                    status = gJavaVM->AttachCurrentThread(&env, NULL);
                    if(status < 0) {
                        LOGE("callback_handler: failed to attach "
                             "current thread");
                        return;
                    }
                    isAttached = true;
                }
                /* Construct a Java string */
                jstring js = env->NewStringUTF(s);
                jclass interfaceClass = env->GetObjectClass(gInterfaceObject);
                if(!interfaceClass) {
                    LOGE("callback_handler: failed to get class reference");
                    if(isAttached) gJavaVM->DetachCurrentThread();
                    return;
                }
                /* Find the callBack method ID */
                jmethodID method = env->GetStaticMethodID(
                    interfaceClass, "callBack", "(Ljava/lang/String;)V");
                if(!method) {
                    LOGE("callback_handler: failed to get method ID");
                    if(isAttached) gJavaVM->DetachCurrentThread();
                    return;
                }
                env->CallStaticVoidMethod(interfaceClass, method, js);
                if(isAttached) gJavaVM->DetachCurrentThread();
            }
              0
              да, воды многовато. Хотя общая идея очень полезная.
              +2
              «В продакшн не попадет» — откуда такое утверждение?
              У нас как раз код с похожей ошибкой мог уйти в продакшн, если бы его замаскировали проверкой и выводом в лог.
              Метод активирующий вибрацию содержал ошибку в названии. На устройствах программистов просто нет вибрации и ошибку никто не засек.
              Засекли, когда на устройстве тестера игра свалилась в исключение.
              Была бы там маскировка и вывод в лог — вполне могли пропустить в продакшн, тесте вполне мог не обратить внимание что вибрация не срабатывает когда должна, а уж лог анализировать в его задачи не входит.
              А так исключение, баг репорт, фикс. Все счастливы.
              Каким образом вы гарантируете, что баг с такой маскировкой не пройдет мимо тестера?
              И вообще, зачем молчать о баге на этапе тестирования?

              По поводу конкретного примера — как я уже сказал и в этом примере маскировка совершенно лишняя.
              Ну и в целом пример значения не имеет. Примеров таких миллиард и простые эникейщики перетаскивают эти првоерки даже не задумываясь как они будут работать в реальном продукте.
                0
                Нужно разделять баги от криворукости программиста.
                Человеку дана задача — прикрутить вибрацию. Он поставил результат данного таска в complete, даже не проверив — это вопрос компетенции. В продакшне этот код (пусть даже УЖЕ) никогда не вызовет ошибку. Значит данные перехваты исключений были написаны лишь для удобства отладки, о чем я и писал. Но компетентному сотруднику будет достаточно и тех же if-return. Я как программист гарантирую, что такого рода баги не дойдут до тестировщика.

                Возможно, вы слишком неудачно выбрали пример.
                Если бы такой if-return применили внутри метода saveUserDataToSdCard(), т.е. данные не записались, а я ничего об этом не знаю — я бы огорчился.

                Повторюсь
                Очевидно, что нужно перехватывать ошибки там, где это нужно.
                Если метод не должен отвечать за определенное поведение — не нужно туда его прибивать.
                  0
                  Ну это клево, когда фирма может для разработки мелких андроид игр нанимать профессионалов с зарплатой в 2-3 000$. Но не все так могут. Гораздо проще научиьт человека выдавать код легко проверяемый тестами, чем нанимать супер профи.

                  Смею предположить что не только мы используем эникейщиков для простых задач… Судя по тому, как много косяк в существующем софте. Видимо программисты пищущие фотошоп, open office и другой софт не настолько круты и не могут гарантировать что их ошибки не дойдут до продакшена. :)
                +2
                Обожаю приложения, которые исходя из этой логики сносят результат работы, потому что не смогли обработать какую-то мелочь. Приложение ошибки может и логгировать, и разработчику слать, но падать — это дурной тон. Особенно, если приложение statefull. Падающий фанарик ещё пережить можно, падающее приложения для фотографий, которое скопычивается не сохранив Главную Фотку Всей Жизни — это подлость и гадость.
                  +2
                  Пусть лучше оно падает, чем делает вид, что все нормально и при этом не сохраняет Главную Фотку Всей Жизни. Имеено об этом автор и попытался рассказать.
                    0
                    Оно не должно падать. Оно должно выдать сообщение «не могу сохранить фото»! Или даже пусть «произошла ошибка доступа к файловой системе. Попробуйте повторить». И параллельно отправить отчет разработчику. Если вы знаете, что если вот тут прилетят кривые данные на вход, то приложение упадёт — надо этот момент обработать, а не оставлять в виде «когда упадёт, тогда и раскопаем».
                      +3
                      Понятно, что так надо, никто и не говорит что так не надо. Однако мы живем в несовершенном мире и вы не представляете сколько раз я видел следующее:
                      1. пустые catch в которых просто ничего не делается
                      2. как в примере логирование и возврат из функции
                      3. просто игнорирование возвращаемого значения (кстати это один из пунктов почему исключения лучше)
                      и тп.

                      Вообще говоря создание приложения которое правильно обрабатывает даже большую часть ошибок и при этом продолжает работать очень не простое дело и что характерно достаточно дорогое.
                      В итоге, выбирая из 2х зол меньшее, лучше пусть программисты вообще не обрабатывают ошибки, чем маскируют их.

                        0
                        Именно так. Мне и добавить нечего. :)
                  +1
                  Как-то статья ни о чем… попробую пересказать кратко:

                  1) не делайте «заглушки» на перехваченные исключения — их надо обработать как-то в логике приложения. Кэп плачет от умиления.
                  2) позволяйте приложению падать по любому поводу. Тут не согласен — нужно не падать, а слать сообщение разработчику, плюс первый пункт — как-то отработать такую ситуацию, а не просто её игнрировать.
                  • UFO just landed and posted this here
                      +2
                      Как правило, заглушки как раз и появляются, когда программист пытается «как-то отработать». Пишется код вызова какой-то функции, по описанию видится, что она какое-то там исключение генерирует, пишется тривиальный обработчик этого исключения в надежде актуализировать его потом… и тут релиз.

                      Соответственно, слать разработчику, если это возможно, надо, но делать это надо, всё-таки, с последующим повторным киданием исключения.
                      А препятствовать всплыванию исключения только в том случае, если точно знаешь, что это за исключение такое и чем его игнорирование/обработка в данном месте грозит.
                      +3
                      В свое время для своих команд я разрабатывал стандарты кодирования, включая как правильно использовать исключения. По итогу мы пришли к двум пунктам:

                      — использовать исключения только для исключительных ситуаций, а не для логики. Например, правильно — проверить существование файла и открывать его, иначе писать сообщение об ошибке (для ситуаций, когда файл может не существовать на диске, например, если мы используем данные, введенные пользователем). Неправильно — сразу пытаться открыть файл, и проверять существование файла обработчиком исключений. Конечно, это очень соблазнительно — все сконцентрировать в одном месте, однако это чревато для сопровождения, потому что будущие разработчики могут подумать, что в данной функции всегда предполагается существование файла. То же самое касается проверки кодов ошибок — если проверка является частью логики (например, для программирования OpenSSL это единственный вариант), тогда надо ее использовать. Если нет — тогда позволять возникать исключениям.

                      — исключения должны перехватываться только там, где мы знаем, что с ними делать. Писать в лог только для того, что бы что-то написать практически бесполезно, и в основном используется только для отладки. Правильное решение — это выкидывать исключения наверх до тех пор, пока мы не сможем сделать что-то полезное с ними. Например, мы ничего не сможем сделать с исключением «метод не найден» (потому что это косяк программиста), но мы сможем что-то сделать с исключением «нехватка памяти» (например, выгрузить что-то из памяти, освободить кэши, но для этого надо чтобы менеджер кэшей получил это исключение). Для кодов ошибок это не так тривиально, поэтому иногда приходится создавать собственную иерархию исключений.

                      Теперь конкретно по примерам Android NDK — изначально исключения не поддерживаются, надо специально менять mk-файлы. Также в основном примеры для C, в котором нет исключений. Проверка ошибки в вышеприведенном примере в идеале не нужна, вы правильно отметили, однако она нужна для отладки и в целом для облегчения жизни программистов: JNI — не самая удобная платформа для взаимодействия Java с native-кодом, особенно учитывая магию типа названий native-функций Java_<class_path_goes_here>_<method_name_goes_here>.
                        0
                        Для исключений(именно как средство отлова ошибок) видел вот такой метод:
                        class cExceptionHack{
                        protected:
                        	int					m_Data;
                        public:
                        	void throwException(){
                        		m_Data++;
                        	};
                        };
                        


                        cExceptionHack* Exception = NULL;
                        Exception->throwException();
                        


                        Решение более чем спорное, но интересное и результат дает — на выходе отчет об ошибке с call stack внутри.
                        Хотя мне видится, что включить исключения является значительно более корректным решением.
                          +1
                          Например, правильно — проверить существование файла и открывать его, иначе писать сообщение об ошибке (для ситуаций, когда файл может не существовать на диске, например, если мы используем данные, введенные пользователем). Неправильно — сразу пытаться открыть файл, и проверять существование файла обработчиком исключений.


                          В момент между проверкой на существование и чтением файл вполне может исчезнуть. Например, пользователь удалил файл или вообще выдернул диск. Так что обрабатывать такие исключения нужно в любом случае.
                          Кстати, именно поэтому в Java исключения делятся на проверяемые и непроверяемые. Причину непроверяемого исключения (например, нулевую ссылку) можно проверить в самой программе. Поэтому в нормальных условиях они возникать не должны. Проверяемые же исключения таким образом предупредить невозможно — они обычно вызваны внешними событиями.
                        +2
                        То, о чем Вы написали, имеет название — Fail fast.
                          0
                          Интересная методика… и так близка сердцу. :)

                        Only users with full accounts can post comments. Log in, please.