-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Показ стека вызовов при ошибках в приложении #8
base: master
Are you sure you want to change the base?
Conversation
Обзор измененийВ последнем обновлении добавлен новый параметр для отображения стека вызовов в функции Изменения
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Классы/ИКС_Рогатка.os (3 hunks)
Additional comments: 1
src/Классы/ИКС_Рогатка.os (1)
- 7-8: Добавление нового параметра
ПоказыватьСтекВызовов
в функциюДеталька
кажется логичным и соответствует описанию в PR. Однако, в предоставленном коде отсутствует сама реализация функцииДеталька
, что затрудняет полное понимание контекста использования нового параметра. Рекомендуется убедиться, что изменения в функцииДеталька
корректно обрабатывают новый параметр и не нарушают обратную совместимость.
src/Классы/ИКС_Рогатка.os
Outdated
Если ПоказыватьСтекВызовов Тогда | ||
ПодробныйСтекВызовов = СтекВызововСтрокой(ИнформацияОбОшибке()); | ||
Сообщить("Стек трейс: | ||
|" + ПодробныйСтекВызовов); | ||
КонецЕсли; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Изменения в функции ПриЗапускеПриложения
для отображения стека вызовов при ошибке корректно реализованы. Использование условия Если ПоказыватьСтекВызовов Тогда
и последующее формирование сообщения со стеком вызовов соответствует задаче. Однако, стоит обратить внимание на многострочный литерал в строках 20-21, который может быть заменен на использование функции СтрШаблон
для улучшения читаемости кода.
- Сообщить("Стек трейс:
- |" + ПодробныйСтекВызовов);
+ Сообщить(СтрШаблон("Стек трейс:%1|%2", Символы.ПС, ПодробныйСтекВызовов));
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Если ПоказыватьСтекВызовов Тогда | |
ПодробныйСтекВызовов = СтекВызововСтрокой(ИнформацияОбОшибке()); | |
Сообщить("Стек трейс: | |
|" + ПодробныйСтекВызовов); | |
КонецЕсли; | |
Если ПоказыватьСтекВызовов Тогда | |
ПодробныйСтекВызовов = СтекВызововСтрокой(ИнформацияОбОшибке()); | |
Сообщить(СтрШаблон("Стек трейс:%1|%2", Символы.ПС, ПодробныйСтекВызовов)); | |
КонецЕсли; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это менее читабельно, Кролик
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nixel2007, понимаю вашу точку зрения и согласен, что читаемость кода может варьироваться в зависимости от предпочтений разработчика и контекста кодовой базы. Важно поддерживать стиль, который вы считаете наиболее подходящим для вашего проекта. Если у вас возникнут дополнительные вопросы или потребуется помощь, я всегда готов помочь.
src/Классы/ИКС_Рогатка.os
Outdated
СтекВызовов_ = ИнформацияОбОшибке.ПолучитьСтекВызовов(); | ||
МассивТекстИсключения = Новый Массив; | ||
|
||
Отступ = ""; | ||
СимволОтступа = " "; | ||
ПервыйКадр = Истина; | ||
Для Каждого КадрСтекаВызовов Из СтекВызовов_ Цикл | ||
// пропустим первый кадр стека, т.к. он ведет на служебный вызов исключения | ||
Если ПервыйКадр Тогда | ||
ПервыйКадр = Ложь; | ||
Продолжить; | ||
КонецЕсли; | ||
|
||
Отступ = Отступ + СимволОтступа; | ||
|
||
СтрокаСтекТрейса = СтрШаблон( | ||
"%1%2 / Метод %3 / Строка %4", | ||
Отступ, | ||
КадрСтекаВызовов.ИмяМодуля, | ||
КадрСтекаВызовов.Метод, | ||
Формат(КадрСтекаВызовов.НомерСтроки, "ЧГ=") | ||
); | ||
МассивТекстИсключения.Добавить(СтрокаСтекТрейса); | ||
КонецЦикла; | ||
|
||
Возврат СтрСоединить(МассивТекстИсключения, Символы.ПС); | ||
|
||
КонецФункции |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Функция СтекВызововСтрокой
корректно реализована для форматирования стека вызовов в строку. Использование цикла для обхода каждого кадра стека вызовов и пропуск первого кадра является правильным подходом. Однако, стоит обратить внимание на использование переменной Отступ
для формирования отступов в стеке вызовов. Это может привести к неоптимальной производительности при большом количестве кадров стека из-за постоянного пересоздания строки. Рассмотрите возможность оптимизации этого аспекта.
- Отступ = Отступ + СимволОтступа;
+ Если Не ПервыйКадр Тогда
+ Отступ = Отступ + СимволОтступа;
+ КонецЕсли;
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Функция СтекВызововСтрокой(Знач ИнформацияОбОшибке) Экспорт | |
СтекВызовов_ = ИнформацияОбОшибке.ПолучитьСтекВызовов(); | |
МассивТекстИсключения = Новый Массив; | |
Отступ = ""; | |
СимволОтступа = " "; | |
ПервыйКадр = Истина; | |
Для Каждого КадрСтекаВызовов Из СтекВызовов_ Цикл | |
// пропустим первый кадр стека, т.к. он ведет на служебный вызов исключения | |
Если ПервыйКадр Тогда | |
ПервыйКадр = Ложь; | |
Продолжить; | |
КонецЕсли; | |
Отступ = Отступ + СимволОтступа; | |
СтрокаСтекТрейса = СтрШаблон( | |
"%1%2 / Метод %3 / Строка %4", | |
Отступ, | |
КадрСтекаВызовов.ИмяМодуля, | |
КадрСтекаВызовов.Метод, | |
Формат(КадрСтекаВызовов.НомерСтроки, "ЧГ=") | |
); | |
МассивТекстИсключения.Добавить(СтрокаСтекТрейса); | |
КонецЦикла; | |
Возврат СтрСоединить(МассивТекстИсключения, Символы.ПС); | |
КонецФункции | |
Функция СтекВызововСтрокой(Знач ИнформацияОбОшибке) Экспорт | |
СтекВызовов_ = ИнформацияОбОшибке.ПолучитьСтекВызовов(); | |
МассивТекстИсключения = Новый Массив; | |
Отступ = ""; | |
СимволОтступа = " "; | |
ПервыйКадр = Истина; | |
Для Каждого КадрСтекаВызовов Из СтекВызовов_ Цикл | |
// пропустим первый кадр стека, т.к. он ведет на служебный вызов исключения | |
Если ПервыйКадр Тогда | |
ПервыйКадр = Ложь; | |
Продолжить; | |
КонецЕсли; | |
Если Не ПервыйКадр Тогда | |
Отступ = Отступ + СимволОтступа; | |
КонецЕсли; | |
СтрокаСтекТрейса = СтрШаблон( | |
"%1%2 / Метод %3 / Строка %4", | |
Отступ, | |
КадрСтекаВызовов.ИмяМодуля, | |
КадрСтекаВызовов.Метод, | |
Формат(КадрСтекаВызовов.НомерСтроки, "ЧГ=") | |
); | |
МассивТекстИсключения.Добавить(СтрокаСтекТрейса); | |
КонецЦикла; | |
Возврат СтрСоединить(МассивТекстИсключения, Символы.ПС); | |
КонецФункции |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Количество аллокаций строк в обоих вариантах одинаковое
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nixel2007, спасибо за ваш комментарий. Вы правы, что предложенное изменение не уменьшает количество аллокаций строк, так как операция конкатенации все равно происходит в цикле для каждого кадра стека вызовов, за исключением первого. Моя цель была предложить оптимизацию, но ваше замечание подчеркивает, что в данном контексте оно не приводит к существенному изменению в плане аллокаций. Ценю ваш вклад и внимание к деталям.
остается только прикладной стек приложения, без служебного стека ОСени вывод через Лог
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Классы/ИКС_Рогатка.os (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/Классы/ИКС_Рогатка.os
замена на ПоказыватьСтекВызововПриОшибкеПриОшибке
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Классы/ИКС_Рогатка.os (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/Классы/ИКС_Рогатка.os
src/Классы/ИКС_Рогатка.os
Outdated
Лог.Ошибка("Стек трейс: | ||
|" + ПодробныйСтекВызовов); | ||
КонецЕсли; | ||
// основную ошибку покажем ниже стека вызовов, чтобы пользователю не пришлось скроллить | ||
Лог.Ошибка(ОписаниеОшибки()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А зачем и то и то показывать? Разве у стек трейсе не будет описания ошибки? Если нет, то лучше этот вывод склеить в одну строку и вывести одновременно, так как между строками лога может вмешаться вывод из других потоков.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А зачем и то и то показывать? Разве у стек трейсе не будет описания ошибки? Если нет, то лучше этот вывод склеить в одну строку и вывести одновременно, так как между строками лога может вмешаться вывод из других потоков.
1 ага, в стек-трейсе не будет описания ошибки
2 лады, склею также, как сейчас
- сначала стек
- затем финальная ошибка - так будет понятнее пользователю и меньше скроллить
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
склеил
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Классы/ИКС_Рогатка.os (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/Классы/ИКС_Рогатка.os
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- README.md (12 hunks)
Additional comments: 5
README.md (5)
- 38-44: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [41-50]
Изменения в аннотациях и описаниях для команды "plus" выглядят корректно.
- 54-58: Уточнения в описании механизма работы с командами и аргументами полезны для понимания.
- 74-80: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [77-86]
Изменения в описании опций команды "hello" корректны и способствуют лучшему пониманию.
- 100-106: Описание механизма работы с подкомандами через аннотации улучшает документацию.
- 230-253: Описание механизма включения и отключения показа стека вызовов при ошибках улучшает документацию и способствует лучшему пониманию работы с ошибками.
# autumn-cli | ||
|
||
Обертка над [cli](https://github.com/khorevaa/cli) библиотекой, которая предоставляет возможности создания консольных приложений на фреймворке [ОСень](https://github.com/autumn-library/autumn). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [152-152]
В описании параметров аннотаций предложение начинается с маленькой буквы. Рекомендуется исправить для улучшения читаемости.
- параметр повторяемый, т.е. может быть несколько подкоманд.
+ Параметр повторяемый, т.е. может быть несколько подкоманд.
КонецФункции | ||
``` | ||
|
||
## Настройка показа полного стека вызовов при ошибках приложения | ||
|
||
При ошибках, исключениях внутри приложения вместе с описанием ошибки по умолчанию выдается полный стек вызовов приложения. | ||
Стек служебных вызовов из библиотек ОСени исключается для облегчения чтения логов пользователем. | ||
|
||
Чтобы исключить показ стека вызовов при ошибках, нужно | ||
|
||
- в файле `autumn-properties.json` указать значение `false` для ключа `ПоказыватьСтекВызововПриОшибке` в корневом разделе `cli` | ||
- пример файла | ||
```json | ||
{ | ||
"cli": { | ||
"ИмяПриложения": "cli_test", | ||
"ПолноеИмяПриложения": "cli_test v%{cli.ВерсияПриложения}", | ||
"ВерсияПриложения": "1.0.1", | ||
"ПоказыватьСтекВызововПриОшибке" : false | ||
} | ||
} | ||
``` | ||
|
||
Чтобы включить показ стека вызовов при ошибках, можно | ||
- или задать `true` | ||
- или просто удалить ключ из файла для возврата поведения по умолчанию. | ||
|
||
|
||
## Миграция с [cli](https://github.com/khorevaa/cli) | ||
|
||
Инфраструктурный, условный ```main.os``` от вашего приложения не нужен. он должен быть запущен при помощи [ОСени](https://github.com/autumn-library/autumn). Дальше перед вами три пути: | ||
Инфраструктурный, условный ```main.os``` от вашего приложения не нужен. он должен быть запущен при помощи [ОСени](https://github.com/autumn-library/autumn). Дальше перед вами три пути: | ||
|
||
Первый - команды модифицировать аннотациями конструктора ```&КомандаПриложения``` или ```&ПодкомандаПриложения```. оставить методы ```ОписаниеКоманды``` и ```ВыполнитьКоманду```, тогда фреймворк поймет что они уже реализованы, и не будет генерировать для них код. | ||
Первый - команды модифицировать аннотациями конструктора ```&КомандаПриложения``` или ```&ПодкомандаПриложения```. оставить методы ```ОписаниеКоманды``` и ```ВыполнитьКоманду```, тогда фреймворк поймет что они уже реализованы, и не будет генерировать для них код. | ||
|
||
Второй - команды модифицировать аннотациями конструктора ```&КомандаПриложения``` или ```&ПодкомандаПриложения```. убрать методы ```ОписаниеКоманды``` и ```ВыполнитьКоманду```, и создать поля класса, с необходимыми аннотациями, и метод с аннотацией ```&ВыполнениеКоманды```. В таком случае фреймворк сгенерит все остальное за вас. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [257-263]
В описании миграции с библиотеки cli есть предложения, начинающиеся с маленькой буквы. Рекомендуется исправить для улучшения читаемости.
- он должен быть запущен при помощи [ОСени](https://github.com/autumn-library/autumn).
+ Он должен быть запущен при помощи [ОСени](https://github.com/autumn-library/autumn).
- оставить методы ```ОписаниеКоманды``` и ```ВыполнитьКоманду```, тогда фреймворк поймет что они уже реализованы, и не будет генерировать для них код.
+ Оставить методы ```ОписаниеКоманды``` и ```ВыполнитьКоманду```, тогда фреймворк поймет что они уже реализованы, и не будет генерировать для них код.
- убрать методы ```ОписаниеКоманды``` и ```ВыполнитьКоманду```, и создать поля класса, с необходимыми аннотациями, и метод с аннотацией ```&ВыполнениеКоманды```.
+ Убрать методы ```ОписаниеКоманды``` и ```ВыполнитьКоманду```, и создать поля класса, с необходимыми аннотациями, и метод с аннотацией ```&ВыполнениеКоманды```.
src/Классы/ИКС_Рогатка.os
Outdated
КонецЕсли; | ||
|
||
ЭтоКадрСтекаИзОсени = СтрНачинаетсяС(КадрСтекаВызовов.Метод, "ДекораторВнутренний_") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тянет на отдельный метод
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
согласен, выделяю отдельный метод ЭтоКадрСтекаИзОсени(КадрСтекаВызовов)
…араметр убрал ненужный Экспорт
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Классы/ИКС_Рогатка.os (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/Классы/ИКС_Рогатка.os
Функция ЭтоКадрСтекаИзОсени(КадрСтекаВызовов) | ||
Возврат СтрНачинаетсяС(КадрСтекаВызовов.Метод, "ДекораторВнутренний_") | ||
И СтрНачинаетсяС(КадрСтекаВызовов.ИмяМодуля, "<string"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это не всегда так. Это может быть пользовательский декоратор, созданный пользовательским напильником
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@artbear получится поковырять ещё?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется, можно ограничиться классом ЗапускательПриложения
Summary by CodeRabbit
Новые функции
Документация