Skip to content

Conversation

HeroProtagonist
Copy link
Contributor

resolves #774

  • captureMessage will now respect ignoreUrls/whitelistUrls
  • An error will always be generated in captureMessage. The stack trace of this error will be used to test the file url and see if it matches ignoreUrls/whitelistUrls.
    • The error will still only be added to the payload if the configurations specifies this
  • Added tests
  • Updated docs to reflect the change

docs/config.rst Outdated
filter out any values.

Does not affect ``captureMessage`` or when non-error object is passed in
Does not affect when non-error object is passed in
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't be true anymore with this PR, as Raven.captureException({foo: 'bar'}), will end up calling Raven.captureMessage https://github.com/getsentry/raven-js/blob/master/src/raven.js#L378-L389

docs/config.rst Outdated
regular expressions or strings.

Does not affect captureMessage or when non-error object is passed in
Does not affect when non-error object is passed in
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

docs/config.rst Outdated
}
Does not affect captureMessage or when non-error object is passed in
Does not affect when non-error object is passed in
Copy link
Contributor

Choose a reason for hiding this comment

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

And here

src/raven.js Outdated
if (
!!this._globalOptions.ignoreUrls.test &&
this._globalOptions.ignoreUrls.test(fileurl)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrap return statement in curly braces

!!this._globalOptions.whitelistUrls.test &&
!this._globalOptions.whitelistUrls.test(fileurl)
)
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

src/raven.js Outdated
ex.name = null;
var stack = TraceKit.computeStackTrace(ex);

var prvCall = stack.stack[1];
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd call this initialCall to give it more meaning. Also please add comment about why it's 2nd frame, not the top one – similar to here https://github.com/getsentry/raven-node/blob/master/lib/client.js#L337

Raven._globalOptions.whitelistUrls = joinRegExp([/.+?host1.+/, /.+?host2.+/]);
TraceKitStub.returns({stack: [{url: 'http://host1/'}, {url: 'http://host1/'}]});
Raven.captureMessage('Capture!');
assert.equal(Raven._send.callCount, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use assert.isTrue(Raven._send.calledOnce); and assert.isTrue(Raven._send.calledTwice); to be more consistent with the next test (which uses callOnce attribute).

@kamilogorek
Copy link
Contributor

Thanks for contributing @HeroProtagonist! Just a few minor requests and we're good to go :)

@HeroProtagonist
Copy link
Contributor Author

@kamilogorek Thanks for the review! I have updated my PR reflecting your comments

@kamilogorek kamilogorek merged commit d7beae8 into getsentry:master Oct 10, 2017
@kamilogorek
Copy link
Contributor

Aaaand it's merged! Thanks @HeroProtagonist! :)

@benvinegar
Copy link
Contributor

🙌 Huge!

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.

captureMessage should respect ignoreUrls, whitelistUrls

3 participants