Skip to content
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

Next level! #25

Closed
wants to merge 1 commit into from
Closed

Next level! #25

wants to merge 1 commit into from

Conversation

vs-kurkin
Copy link

Предпосылки для изменений:

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

Changelog:

  • Terror теперь может принимать вторым аргументом объект error.
  • Если вторым аргументом была передана строка, она не попадает в свойство originalError. Существуют сторонние обертки над ошибками, использующие значение из этого свойства. Если в нем содержится что-то отличное от error, возникало исключение при работе с terror.originalError.stack.
  • Свойство Terror#message содержит только оригинальное сообщение текущего объекта.
  • Новое свойство Terror.stackTraceLimit позволяет ограничивать глубину формируемого стека ошибки (если поддерживается интерпретатором). Может быть переопределено для любого конструктора, наследуемого от Terror. С помощью данного свойства можно сильно снизить нагрузку на процессор, если устанавить его значение в 0 на высоких уровнях приложения.
  • Terror.isTerror теперь не падает, если передать null или undefined.
  • Terror#log теперь логирует стеки всех обернутых ошибок.
  • Новый метод Terror#getFullMessage возвращает полное сообщение об ошибке, включая сообщения обернутых экземпляров. Сообщение каждого объекта ошибки воспринимается как предложение. Формат возвращаемого значения: ERR_CODE Can not create page. Can not instance widget. a is no a function.
  • Новый метод Terror#getFullStack возвращает стеки всех обернутых экземпляров. Каждый последующий стек отбивается четыремя пробелами слева (стандартный отступ стека). К каждому сообщению об ошибке добавляется код.
  • Terror#setMessage теперь не меняет сообщение в сформированном стеке.
  • Кастомные конструкторы теперь имеют полноценное имя (MyError.name).
  • Оптимизация производительности: cd benchmark; npm test.
  • Тесты переписаны на jasmine.
  • Terror теперь не зависит от NodeJS и может выполняться в любом окружении (Issuse #1).
  • Мог что-то упустить.

Что нужно сделать:

  • Портировать тесты на mocha, раз вы используете его. Удалить старые тесты.
  • Решить с форматом Terror#getFullStack, так как он сейчас жестко зашит в метод. Я бы оставил и так, ничего криминального в этом нет. Terror#logMultilineError тоже этим грешит.
  • Обновить README.md и JSDoc. Мой английский не позволяет это сделать грамотно.
  • Что-нибудь еще?

@vs-kurkin
Copy link
Author

Упс... fatal: unable to connect to github.com.

Terror.create = function (name, codes) {
var Inheritor = new Function('Terror', sPrintF(SOURCE_CONSTRUCTOR, {
name: name
}))(Terror);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я правильно понимаю, что единственный смысл этого — иметь именованный конструктор от name?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Функционально - да.
Так же есть небольшой профит в производительности, поскольку нет необходимости рекурсивно вызывать все конструкторы в цепочке наследования. Сейчас вызывается только текущий конструктор и Terror. Этого можно было бы добиться и с замыканием, но я их не люблю.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Ты правда смог это воспроизвести на benchmark'ах? Или это видно по IR? Я конечно не копался очень детально, но разницы вообще никакой не увидел

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@kaero
Copy link
Contributor

kaero commented Mar 30, 2015

@B-Vladi squash commits, pls. I'll merge PR into temporary v1.2 branch to update tests and doc.

@vs-kurkin vs-kurkin force-pushed the master branch 4 times, most recently from ed3f4db to d3ada9c Compare March 30, 2015 09:58
@vs-kurkin
Copy link
Author

Done

@kaero
Copy link
Contributor

kaero commented Mar 30, 2015

@an9eldust as I can remember, you was interested in Terror implementation for client-side. This PR provides some support, except IE. Are you still interested in it?

@vs-kurkin
Copy link
Author

Fixes for old IE

@kaero
Copy link
Contributor

kaero commented Apr 1, 2015

merged manually into branch v1.2

@kaero kaero closed this Apr 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants