Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

feat: Allow to configure stacktrace for captureMessage calls #388

Merged
merged 1 commit into from
Oct 6, 2017

Conversation

kamilogorek
Copy link
Contributor

@kamilogorek kamilogorek commented Oct 5, 2017

var ex;
// Generate a "synthetic" stack trace
try {
throw new Error(message);
Copy link
Member

Choose a reason for hiding this comment

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

do you have to throw the error to get the stack?

Copy link
Contributor Author

@kamilogorek kamilogorek Oct 5, 2017

Choose a reason for hiding this comment

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

No, it's just more explicit, as we wrap it in try clause. Also this is how we have it written in raven-js and I'm trying to write as unified code as possible.

Copy link
Member

@mitsuhiko mitsuhiko Oct 5, 2017

Choose a reason for hiding this comment

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

I'm mostly asking because throwing an exception vs just creating an error should have noticable performance differences. And since message logging is potentially more common than crashes it might be a concern here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm surprised with this, but you're right : o

throw new Error(i)
608.497162ms
575.068622ms
574.888692ms
540.633688ms
540.919783ms
550.634267ms
577.011109ms
541.462129ms
568.272018ms
573.879691ms

avg: 565.1267161000001ms

new Error(i)
410.160736ms
412.978303ms
423.740643ms
434.467241ms
420.66899ms
426.131056ms
430.911071ms
426.683749ms
429.369119ms
432.251688ms

avg: 424.73625960000004ms
var start = process.hrtime()
var avg = 0 
var n = 10
var sample = 100000

console.log('\nthrow new Error(i)')

for (var j = 0; j < n; j++) {
  for (var i = 0; i < sample; i++) {
    try {
      throw new Error(i)
    } catch (e) {}
  }

  end = process.hrtime(start)[1]
  start = process.hrtime()

  avg += end 

  console.log('' + (end/1000000) + 'ms')
}
console.log('\navg: ' + (avg/n/1000000) + 'ms')

console.log('\nnew Error(i)')

avg = 0 

for (var j = 0; j < n; j++) {
  for (var i = 0; i < sample; i++) {
    try {
      new Error(i)
    } catch (e) {}
  }

  end = process.hrtime(start)[1]
  start = process.hrtime()

  avg += end 

  console.log('' + (end/1000000) + 'ms')
}
console.log('\navg: ' + (avg/n/1000000) + 'ms')
```js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tests are failing without throw, in the end it is required ¯_(ツ)_/¯

@kamilogorek kamilogorek force-pushed the message-stacktrace branch 3 times, most recently from 316dac8 to 19a8caa Compare October 6, 2017 10:08
@kamilogorek kamilogorek merged commit 31e58d3 into master Oct 6, 2017
@kamilogorek kamilogorek deleted the message-stacktrace branch October 6, 2017 10:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants