Skip to content

Compare ignoreErrors pattern to error message #1076

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 10, 2017
Merged

Compare ignoreErrors pattern to error message #1076

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7a9c259
Compare ignoreErrors pattern to error message
Oct 5, 2017
c90dd1e
Don't prepend a blank type to testString
Oct 6, 2017
5b5689c
Add unit tests
Oct 6, 2017
6acaf22
Clean up unit test
Oct 6, 2017
ce3ce08
Remove the loop
Oct 6, 2017
c933d1a
Combine the if and the else-if
Oct 6, 2017
78e2e77
Simplify
Oct 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/raven.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1423,8 +1423,9 @@ Raven.prototype = {
var testString = (type || '') + ': ' + (message || '');

if (
!!this._globalOptions.ignoreErrors.test &&
this._globalOptions.ignoreErrors.test(testString)
!!this._globalOptions.ignoreErrors.test && (
this._globalOptions.ignoreErrors.test(message) ||
this._globalOptions.ignoreErrors.test(testString))
Copy link
Contributor

@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.

Can we also change it so that testString doesn't become :message if it's undefined? per #1075 (comment)

Copy link
Contributor Author

@staticshock staticshock Oct 6, 2017

Choose a reason for hiding this comment

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

Yep! Addressed.

Copy link
Contributor

@kamilogorek kamilogorek Oct 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This code will double-check the same string if type is not defined, but I think we can live with it.

Although an alternative would be something like this, which also do the job, but doesn't improve a readability really

if (!!this._globalOptions.ignoreErrors.test) {
  var testString = (type ? type + ': ' : '') + (message || '')
  if (this._globalOptions.ignoreErrors.test(message)) return
  if (testString !== message && this._globalOptions.ignoreErrors.test(testString)) return
}

I can live with the code we have now.

)
return;

Expand Down