Add `ignored_in_project_file_paths` configuration

[pjambet]

Goal

Allow single files to be ignored when grouping errors. This is useful when a single file appears in the stacktrace, and ignoring its parent folder through vendor_paths would cause other siblings that we do not want to ignore in the folder to be ignored.

This gist shows an example where this can happen, when a module is used and wraps actions, in this case with an around_create block.

When needing this, the only option I found was to create a middleware that would inspect the content of the stacktraces in the report and flag lines as not in-project. It felt like a lot of work for a seemingly small task. It seemed like a built-in config would help here!

Design

A new config was added, and to optimize for the include? call when processing stacktraces, we make sure to always convert the input to a set, as to not require users to remember to create a set and simply pass an array.

Changeset

A new config is now available, ignored_in_project_file_paths, it can be used like this:

Bugsnag.configure do |config|
  config.ignored_in_project_file_paths = ["abc/other_lib/ignore_me.rb", "abc/lib/ignore_me.rb", "xyz/lib/ignore_me.rb"]
end

Testing

Automated tests were added

[clr182]

Hi @pjambet,

Thank you for your feedback.

I've raised this feature request with our product team for further consideration. We don't have a firm ETA for this but we'll be sure to keep you posted with any updates.

[gareththackeray]

Hi @pjambet. Thanks again for the submission. An on error callback or a middleware gives you the tools to mark stack frames out of project as required. The vendor_paths config item covers a common use case of excluding 3rd party code.

We don't consider this PR to be a common enough use case to warrant a dedicated config item so I'm going to close the PR.

Apologies for this. I hope you understand we have to weigh up the benefit of increasing features and config items against the cost of maintaining them.

[pjambet]

@gareththackeray Thanks for getting back to me.

Some unsolicited feedback:

Purely from a personal experience, customizing what is "in-project" is crucial to getting the most out of busgnag. It was surprisingly difficult to figure out how to use vendor_paths for that purpose (even though the config documentation mentions here), searching for "in project"—as is essentially hinted by this page that explains how grouping works—yields a lot of results, but none for the vendor_paths docs page. For reference I found out about it by finding these lines in the codebase.

I was not aware of the on_error callback, thanks for mentioning this. That being said the subtitle of that page is somewhat misleading:

In order to quickly reproduce and fix errors, it is often helpful to send additional application-specific diagnostic data to BugSnag.

In this case I would want to customize the existing data being sent, not add anything additional.

Finally, I was surprised to not see any documentation about the creation of custom middlewares. Unless such documentation and I missed it, I think it would be a great addition to the docs.

As a final nitpick, the add_on_error method looked surprising to me, in a way that differs from my understanding of idiomatic ruby. Given that it accepts a single proc argument, it seems like it could be defined as:

def add_on_error(&callback)
  middleware.use(callback)
end

so that it can be used as:

config.add_on_error do |report|
  # ...
end

which happens to be look more like idiomatic ruby to me.

To pass an explicit proc instance, the following would be needed:

callback = proc {|report|  }
Bugsnag.add_on_error(&callback)

And for backwards compatibility , the following would work:

def add_on_error(callback = nil, &block)
  callback = block if block
  middleware.use(callback)
end

so that both versions would achieve the same thing:

config.add_on_error(proc { |report| })
config.add_on_error { |report| }

Hope the feedback is useful.

[gareththackeray]

Thanks @pjambet that is helpful. You're right about the importance of in/out of project. We should emphasise this more in the documentation. Regarding the idiomatic Ruby I will pass that onto our engineers for whom it'll be far more meaningful!