Сокрытие исключений
От: Vasiliy2  
Дата: 22.09.16 11:30
Оценка: 29 (2) +2
Здравствуйте.
Решил глянуть нутрянку CodeJam и обнаружил в пространстве имен IO метод, вызываемый при удалении объекта, содержащий такой вот код:
                try
                {
                    File.Delete(path);
                }
                catch (ArgumentException) { }
                catch (IOException) { }
                catch (UnauthorizedAccessException) { }


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

Но, CodeJam пишут у нас уважаемые люди с проведением кодревью, значит вероятность ошибок низка. Где у меня ошибка в рассуждениях?
Re: Сокрытие исключений
От: Sinix  
Дата: 22.09.16 12:14
Оценка:
Здравствуйте, Vasiliy2, Вы писали:

V>Где у меня ошибка в рассуждениях?


В общем случае всё правильно, проглатывать исключения без логирования не стоит.
Но у нас тут частный случай, в котором сообщить про "нишмогла" не так уж и критично.

Вот примерный сценарий использования:
using (var t = TempData.CreateFile())
{
  ... // полезный код
} // пытаемся грохнуть файл


Смотрим, какие исключения тут ловятся:

1. ArgumentException — в теории, его вообще не надо ловить, т.к. оно и бросаться-то не должно. Изначально я планировал, что путь проверяется в момент создания и за владение файлом отвечает сам код в TempData, но тут был наглый поклёп, убрал всё пошло слегка не так, как задумано Момент спорный, обсудим и починим.

Тем не менее, исключение было добавлено, т.к.

BRIAN GRUNKEMEYER Note that if your constructor throws an exception, the finalizer for your type will still run! So the finalizer and the Dispose(false) code path in your type must be prepared to handle an uninitialized state. Worse yet, if your app must deal with asynchronous exceptions such as ThreadAbortException or OutOfMemoryException, your finalizer may have to deal with partially initialized state if your constructor threw an exception halfway through! This surprising fact is usually pretty easy to deal with, but it may take you several years to realize it

(с)FDG


2. IOException и UnauthorizedAccessException могут быть брошены во куче причин.
Например, код внутри using открыл файл на чтение, бросил исключение и позабыл закрыть поток в finally.
Если в этом случае просто пробросить исключение "не смог удалить файл", то мы замаскируем исходное исключение.

Подробней — всё тот же FDG:

AVOID throwing an exception from within Dispose(bool) except under critical situations where the containing process has been corrupted (leaks, inconsistentshared state, etc.).
Users expect that a call to Dispose will not raise an exception. For example, consider the manual try-finally in this snippet:

TextReader tr = new StreamReader(File.OpenRead("foo.txt")); try {
// do some stuff
}
finally {
tr.Dispose();
// more stuff
}

If Dispose could raise an exception, further finally-block cleanup logic will not execute.


Вот как-то так. Если честно, этот код писался на автомате и проверки я ставил самые стандартные. Если есть идеи как сделать лучше — вэлкам!
UPD Нагло вру, тысяча извинений. Код в TempData изначально был написан AndrewVK, я его правил, подглядывая в наше аналогичное решение.
А вот его я уже писал на автомате
Отредактировано 22.09.2016 12:51 Sinix . Предыдущая версия . Еще …
Отредактировано 22.09.2016 12:35 Sinix . Предыдущая версия .
Отредактировано 22.09.2016 12:34 Sinix . Предыдущая версия .
Re[2]: Сокрытие исключений
От: Vasiliy2  
Дата: 22.09.16 12:52
Оценка:
Здравствуйте, Sinix, Вы писали:

S>Здравствуйте, Vasiliy2, Вы писали:


S>В общем случае всё правильно, проглатывать исключения без логирования не стоит.

S>Но у нас тут частный случай, в котором сообщить про "нишмогла" не так уж и критично.

S>Вот примерный сценарий использования:

S>
S>using (var t = TempData.CreateFile())
S>{
S>  ... // полезный код
S>} // пытаемся грохнуть файл
S>


этот сценарий легким движением руки может трансформироваться в:
IEnumerable<T> items //100000 items

foreach(var item in items)
using (var t = TempData.CreateFile())
{
  ... // полезный код
} // пытаемся грохнуть файл


на выходе имеем 100000 временных файлов (или директорий).

S>Вот как-то так. Если честно, этот код писался на автомате и проверки я ставил самые стандартные. Если есть идеи как сделать лучше — вэлкам!


Да мой то вопрос не упрек. Это вопрос.

Насчет как лучше, надо подумать на досуге, но пока без доп вызова внутри using ничего в голову не идет.

Если вэлкам — это в CodeJam Team, то пока не срастается. Только сегодня вообще проект себе открыл почитать.
Re[2]: Сокрытие исключений
От: AndrewVK Россия http://blogs.rsdn.org/avk
Дата: 22.09.16 12:59
Оценка: +1
Здравствуйте, Sinix, Вы писали:

S>UPD Нагло вру, тысяча извинений. Код в TempData изначально был написан AndrewVK


Код AVK потом почти полностью переписали.
... << RSDN@Home 1.0.0 alpha 5 rev. 0 on Windows 8 6.2.9200.0>>
AVK Blog
Re[3]: Сокрытие исключений
От: Sinix  
Дата: 22.09.16 13:05
Оценка:
Здравствуйте, AndrewVK, Вы писали:

AVK>Код AVK потом почти полностью переписали.

Не, но идея-то твоя. А то тут были уже попытки записаться в авторы всея библиотеки
Re[4]: Сокрытие исключений
От: AndrewVK Россия http://blogs.rsdn.org/avk
Дата: 22.09.16 13:11
Оценка:
Здравствуйте, Sinix, Вы писали:

AVK>>Код AVK потом почти полностью переписали.

S>Не, но идея-то твоя.

Тоже не моя. Кто то при обсуждении просил — я просто дернул готовый код из своих проектов.
... << RSDN@Home 1.0.0 alpha 5 rev. 0 on Windows 8 6.2.9200.0>>
AVK Blog
Re[3]: Сокрытие исключений
От: Sinix  
Дата: 22.09.16 13:13
Оценка:
Здравствуйте, Vasiliy2, Вы писали:

V>на выходе имеем 100000 временных файлов (или директорий).

ага, замечание принято

V>Насчет как лучше, надо подумать на досуге, но пока без доп вызова внутри using ничего в голову не идет.

Лично я предлагаю радикальное решение: задокументировать текущее поведение как by design

Если серьёзно, то можно задавать политику обработки в конструкторе или предлагать создавать временные файлы только внутри tempDirectory и падать, если не удаётся очистить папку, но всё это настолько усложняет код, что сам хелпер становится бессмысленным: все грабли, что мы пытались спрятать выходят наружу.

Хорошего решения пока нет, вечером гляну код и попробую боль-менее внятно сформулировать варианты, затем заведу обсуждение в ветке.


V>Если вэлкам — это в CodeJam Team, то пока не срастается. Только сегодня вообще проект себе открыл почитать.

В любом смысле вэлкам. И как пользователь библиотеки — тоже
Re[4]: Сокрытие исключений
От: Vasiliy2  
Дата: 22.09.16 13:17
Оценка:
Здравствуйте, Sinix, Вы писали:

S>В любом смысле вэлкам. И как пользователь библиотеки — тоже


Как пользователь библиотеки — это завсегда, само собой
Re[4]: Сокрытие исключений
От: AndrewVK Россия http://blogs.rsdn.org/avk
Дата: 22.09.16 13:20
Оценка:
Здравствуйте, Sinix, Вы писали:

S>Если серьёзно, то можно задавать политику обработки в конструкторе или предлагать создавать временные файлы только внутри tempDirectory и падать, если не удаётся очистить папку, но всё это настолько усложняет код, что сам хелпер становится бессмысленным: все грабли, что мы пытались спрятать выходят наружу.


Наименьшим злом, видимо, будет необязательный fallback по типу XmlSchemaReader.

V>>Если вэлкам — это в CodeJam Team, то пока не срастается. Только сегодня вообще проект себе открыл почитать.

S>В любом смысле вэлкам. И как пользователь библиотеки — тоже

+1
... << RSDN@Home 1.0.0 alpha 5 rev. 0 on Windows 8 6.2.9200.0>>
AVK Blog
Re[2]: Сокрытие исключений
От: VladD2 Российская Империя www.nemerle.org
Дата: 25.09.16 12:38
Оценка:
Здравствуйте, Sinix, Вы писали:

S>1. ArgumentException — в теории, его вообще не надо ловить, т.к. оно и бросаться-то не должно. Изначально я планировал, что путь проверяется в момент создания и за владение файлом отвечает сам код в TempData, но тут был наглый поклёп, убрал всё пошло слегка не так, как задумано Момент спорный, обсудим и починим.


Т.е. ловим на всякий пожарный?

S>2. IOException и UnauthorizedAccessException могут быть брошены во куче причин.

S>Например, код внутри using открыл файл на чтение, бросил исключение и позабыл закрыть поток в finally.
S>Если в этом случае просто пробросить исключение "не смог удалить файл", то мы замаскируем исходное исключение.

"Забыл" — это уже ошибка. Дайте программисту отловить свою ошибку.

Если программист считает, что это нормально — он сам сделает отлов нужных ему исключений. Не помогайте ему! Вы только помешаете.

Если уж очень хочется, добавьте необязательный параметр в функцию и кому надо его выставит в нужное значение.

Но тупо гасить исключения — это капец! Я такую библиотеку точно использовать не буду.

Отлов исключений — это задача уровня приложения. Не фиг ее в библиотеках решать.
Есть логика намерений и логика обстоятельств, последняя всегда сильнее.
Re[2]: Сокрытие исключений
От: Слава  
Дата: 25.09.16 12:44
Оценка:
Здравствуйте, Sinix, Вы писали:

S>Вот примерный сценарий использования:

S>
S>using (var t = TempData.CreateFile())
S>{
S>  ... // полезный код
S>} // пытаемся грохнуть файл
S>


Добавьте параметр "SupressExceptions" или что-то вроде.
Update: так у функции winapi CreateFile есть флаг FILE_FLAG_DELETE_ON_CLOSE. Зачем это ручное удаление?
Отредактировано 25.09.2016 12:48 Слава . Предыдущая версия .
Re[3]: Сокрытие исключений
От: Sinix  
Дата: 25.09.16 13:49
Оценка:
Здравствуйте, Слава, Вы писали:

С>Update: так у функции winapi CreateFile есть флаг FILE_FLAG_DELETE_ON_CLOSE. Зачем это ручное удаление?

Вот тут
Автор: Sinix
Дата: 24.09.16
обсуждается
Re[3]: Сокрытие исключений
От: Sinix  
Дата: 25.09.16 14:02
Оценка:
Здравствуйте, VladD2, Вы писали:

VD>"Забыл" — это уже ошибка. Дайте программисту отловить свою ошибку.

VD>Если программист считает, что это нормально — он сам сделает отлов нужных ему исключений. Не помогайте ему! Вы только помешаете.

Тож вариант.

Тут в другом проблема: в большинстве случаев исключение будет бросаться, если внутри самого using что-то пошло не так. Получается, исключение "не могу удалить файл" замаскирует исходное. Т.е. выбираем между "остался файл в временной папке" и "остался файл в временной папке + потеряли исходную ошибку". Я пока склоняюсь к первому.


VD>Но тупо гасить исключения — это капец! Я такую библиотеку точно использовать не буду.

Оно много где по умолчанию проглатывается, биндинг, таски и фоновые потоки как пример. Ну, т.е. ужас — да, но точно не ужас-ужас-ужас

P.S. Если можно — перетащи эту ветку вот сюда
Автор: Vasiliy2
Дата: 22.09.16
Чтоб все аргументы за и против вместе были.
Re[4]: Сокрытие исключений
От: VladD2 Российская Империя www.nemerle.org
Дата: 25.09.16 14:37
Оценка:
Здравствуйте, Sinix, Вы писали:

S>Тут в другом проблема: в большинстве случаев исключение будет бросаться, если внутри самого using что-то пошло не так. Получается, исключение "не могу удалить файл" замаскирует исходное. Т.е. выбираем между "остался файл в временной папке" и "остался файл в временной папке + потеряли исходную ошибку". Я пока склоняюсь к первому.


Что значит потерял? Если вы же ему не поможете это сделать, он и не потеряет. Если он что-то потерял, значит кто-то исключение отловил и "заныкал". Если это он сам, то он ССЗБ. Если это вы — он потратит время на поиски и будет сильно недоволен вами, когда найдет причину.

S>Оно много где по умолчанию проглатывается, биндинг, таски и фоновые потоки как пример. Ну, т.е. ужас — да, но точно не ужас-ужас-ужас


И что в этом хорошего? Я вот бушусь от этого биндинга, когда только по анализу Оутпута можно понять, что где просходит.

Более того. Стараюсь работать с включенным перехватом возбуждения исключений, чтобы в зачатке баги ловить.

S>P.S. Если можно — перетащи эту ветку вот сюда
Автор: Vasiliy2
Дата: 22.09.16
Чтоб все аргументы за и против вместе были.


Куда перетащить? Если ты имеешь в виду, перенести подветку в другую тему, то это невозможно. Можно только выделить подветку в отдельную тему и перенести ее между форумами.
Есть логика намерений и логика обстоятельств, последняя всегда сильнее.
Re[5]: Сокрытие исключений
От: Sinix  
Дата: 25.09.16 15:31
Оценка:
Здравствуйте, VladD2, Вы писали:

VD>Что значит потерял?

Я в первом ответе это дело уже расписывал, если коротко —
try
{
  using (GempFile())
  {
    // Something throws Exception1
  } // Dispose() throws Exception2
}
catch (Exception ex)
{
  // Exception2 wins
}


VD>Куда перетащить? Если ты имеешь в виду, перенести подветку в другую тему, то это невозможно. Можно только выделить подветку в отдельную тему и перенести ее между форумами.

Понял, спасиб
Re: Сокрытие исключений
От: Pavel Dvorkin Россия  
Дата: 25.09.16 15:54
Оценка: +1
Здравствуйте, Vasiliy2, Вы писали:

V>
V>                try
V>                {
V>                    File.Delete(path);
V>                }
V>                catch (ArgumentException) { }
V>                catch (IOException) { }
V>                catch (UnauthorizedAccessException) { }
V>


В применении к данному конкретному случаю — обычное отношения к временным файлам. Удалили — хорошо, не получилось — черт с ним.

Делают такое не только на дотнете. Вот сейчас у меня в C:\Users\dvorkin\AppData\Local\Temp\ 616 Мбайт какого-то мусора

А теперь 493 Кбайта. Остальное стер сейчас.

С другой стороны, а делать-то что ? Сообщить юзеру, что не удалось стереть временный файл ? Он очень рад будет это узнать . Что еще можно сделать ?
With best regards
Pavel Dvorkin
Отредактировано 25.09.2016 15:56 Pavel Dvorkin . Предыдущая версия .
Re[2]: Сокрытие исключений
От: Слава  
Дата: 25.09.16 16:12
Оценка:
Здравствуйте, Pavel Dvorkin, Вы писали:

PD>С другой стороны, а делать-то что ? Сообщить юзеру, что не удалось стереть временный файл ? Он очень рад будет это узнать . Что еще можно сделать ?


Да не должно вообще возникать такого! Если файл временный, то он создается с FILE_FLAG_DELETE_ON_CLOSE. А если программа сама, извиняюсь, насрала 100500ГБ мусора временными файлами, сама не смогла его удалить и сама же жалуется, что кончилось место — это ненормально, это совершенно идиотская ситуация.

Какая-то возможность отслеживания ошибок должна присутствовать. Во-первых для лога, во-вторых, можно эти же файлы пытаться удалить при завершении работы программы.
Re[6]: Сокрытие исключений
От: VladD2 Российская Империя www.nemerle.org
Дата: 25.09.16 17:27
Оценка:
Здравствуйте, Sinix, Вы писали:

VD>>Что значит потерял?

S>Я в первом ответе это дело уже расписывал, если коротко —
S>
S>try
S>{
S>  using (GempFile())
S>  {
S>    // Something throws Exception1
S>  } // Dispose() throws Exception2
S>}
S>catch (Exception ex)
S>{
S>  // Exception2 wins
S>}
S>


Это ошибка программиста. Надо было try/catch внутрь using брать. И вы эту ошибку от него скроете.

Правда темп-файлы это тоже не особая проблема. Потом прочистить и все. Н
Есть логика намерений и логика обстоятельств, последняя всегда сильнее.
Re[7]: Сокрытие исключений
От: Sinix  
Дата: 25.09.16 18:40
Оценка:
Здравствуйте, VladD2, Вы писали:

VD>Это ошибка программиста. Надо было try/catch внутрь using брать. И вы эту ошибку от него скроете.

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

VD>Правда темп-файлы это тоже не особая проблема. Потом прочистить и все.

Ну так об этом и вопрос
Автор: Sinix
Дата: 24.09.16
, только отвечать никто что-то не торопится.
Re[4]: Сокрытие исключений
От: koandrew Канада http://thingselectronic.blogspot.ca/
Дата: 30.09.16 14:59
Оценка: +1
Здравствуйте, Sinix, Вы писали:

S>Хорошего решения пока нет, вечером гляну код и попробую боль-менее внятно сформулировать варианты, затем заведу обсуждение в ветке.


Я в таких случаях обычно в процессе инициализации приложения) выполняю СнестиВсеВременныеФайлыПоМаске(), при этом все временные файлы именую с уникальным префиксом (если пишу в системную помойку %TEMP%, и по какой-то причине нельзя завести выделенную папку для такого дела, скажем в AppData). Почему именно при запуске — всё просто, именно при запуске внимание пользователя максимально сфокусировано на программе, и потому в случае ошибки пользователь с бОльшей вероятностю заметит ошибку и примет какие-то меры. На ошибки при завершении программы мало кто обращает внимание, потому чт с точки зрения юзера он уже получил от программы всё, что ему нужно, а потому если она и ругается — да на здоровье, я в таких случаях часто просто прибиваю процесс.
[КУ] оккупировала армия.
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.