Cannot import Bugsnag from 'bugsnag' in test environment

[flexyford]

I'm using the Bugsnag.notify API and therfore have to

import Bugsnag from 'bugsnag';

However, this breaks my tests:

Chrome 43.0 - Global error: Uncaught Error: Could not find module bugsnag imported from

Any suggestions here? I'm curious why index.js explicitly does not include bugsnag in test environments.

// index.js
this._includeBugsnag =
      this.isDevelopingAddon() || process.env.EMBER_ENV !== 'test';

Any chance we could import just the bugsnag/shim in test environments?

[ghost]

Hi @flexyford, thanks for reporting. Can you show me an example test that reproduces the problem and a bit more information about where you're doing the import in your app. e.g. is it in an initializer? Any example files you can provide are helpful

[flexyford]

Hi @binhums thanks for the quick reply. I want to throw an exception, log it to bugsnag, and catch it, without terminating the program execution. Here's a dummy example of what I'm trying to accomplish:

// my-app/components/some-component.js
import Ember from 'ember';
import Bugsnag from 'bugsnag';

export default Ember.Component.extend({
  didInsertElement() {
    this._super(...arguments);

    // Bugsnag Telemetry
    try {
        throw new BugsnagException(
          "Expect this to log a Bugsnag Exception without terminating program execution"
        );
      }
    } catch(e) {
      Bugsnag.notify(e.name, e.message);
    }
  }
}

function BugsnagException(message) {
   this.name = 'Bugsnag Exception';
   this.message = message;
}

I would like to have bugsnag.js in my test environment.

I do not see a reason to exclude bugsnag in test env since notifyReleaseStages: [ 'production' ] would block any messages from being delivered in the test environment anyways.

[ghost]

and to clarify what type of test would this be in? I want to recreate an example where we achieve this, possibly in the dummy test folder for this addon.

[flexyford]

In my instance, it's an acceptance test which uses some-component.js.

I've come up with a solution which I hope still models the intended behavior.

// ember-cli-bugsnag/index.js
// ...
config: function(env, config) {
  const { bugsnag } = config;
  const { releaseStage = env } = bugsnag;

  // Determine if we should include bugsnag:
  // That way if `env = test` it can still include `bugsnag` 
  // if `bugsnag.releaseState !== 'test'`
  this._includeBugsnag = this.isDevelopingAddon() || releaseStage !== 'test';

  return {
    bugsnag: readEnvironmentConfig(process.env)
  };
},

Open to submitting a PR if we decide to go this route

[ghost]

I'm confused why isn't the code in https://github.com/binhums/ember-cli-bugsnag/blob/master/index.js#L45 working in your situation.

[ghost]

It would seem to me that for whatever reason we're explicitly not loading bugsnag in test environments?

[flexyford]

Because I'm in a test environment:

isDevelopingAddon() // => false and process.env.EMBER_ENV === 'test'

So this._includeBugsnag = this.isDevelopingAddon() || process.env.EMBER_ENV !== 'test' // => false

But I want bugsnag to load in my test env

[ghost]

I'm not so good with the pattern matching syntax bear with me. Just so we're clear: so your code snippet overrides this line at the config step based on the release stage specified in the bugsnag config rather than the ember environment?

I'd definitely be open to supporting test environments of course, but I think a solution would be more likely focussed around mocking the Bugsnag library. You wouldn't actually want errors to be reported to bugsnag every time you want your test suite.

My open questions are:

1. why is it explicitly disabled in test mode currently, e.g. performance reasons?
2. the releaseStage is probably not what we're looking for, that is what gets recorded in bugsnag. I.e. this error happened in staging rather than production.
3. does the config hook always get called after the included hook? Is there a way we can make this relationship more verbose?
4. perhaps if test environment is listed in the notifyReleaseStages in the config then we should override the exclusion, but again I don't think you actually want to be hitting bugsnag.

There are examples of what you're describing in the acceptance tests for this addon https://github.com/binhums/ember-cli-bugsnag/blob/master/tests/acceptance/custom-release-stage-test.js. I'm assuming that these work because of the isDevelopingAddon line right?

[ghost]

why is it explicitly disabled in test mode currently, e.g. performance reasons?

to be clear I don't expect you to have an answer to this one :)

[flexyford]

1. My hunch is it's disabled because we do not want to log errors in tests to bugsnag. However, if we exclude test from notifyReleaseStages then this should not be an issue.
2. Okay, that makes more sense. The above code is not the right solution then.
3. In my experience so far, the config hook always gets called BEFORE the included hook. So this._includeBugsnag would already be set.
4. I don't think test should be listed in notifyReleaseStages either.

The examples in acceptance tests work because this addon adds bugsnag as a dependency in package.json.

I think the real mystery is #1: Why is it explicitly disabled?

[ghost]

🕵️ time to pull out the blame hammer

Would you be averse to specifying bugsnag as a dependency in your package.json as a workaround? Not trying to halt this conversation, just trying to help you achieve your immediate goals.

[ghost]

Looks like #52 (they made blame so much better in Github now!):

Approach:

Detect when the host app is in testing and basically make the addon enter stealth mode. Nothing is included in the app/vendor (bugsnag, initializers... nothing)

OK, it doesn't seem like an attempt to stop bugsnag reporting by accident at all costs because surely it isn't reported if it's not in notifyReleaseStages. Could it be performance?

[flexyford]

🤔 I'm sure there are some performance gains by not including it, but it means tests fail when Reporting Handled Bugsnag Exceptions.

The correct solution here might be exposing all Bugnag methods from the ember-cli-bugsnag addon. That way the Custom Diagnostics such as getMetaData are still envoked even when calling Bugsnag.notify or Bugsnag.notifyException.

[flexyford]

Ok I finally figured out what my actual issue was: Reporting Handled Bugsnag Exceptions.

Using the following pattern, I can handle exceptions, pass them to bugnsag using ember-cli-bugsnag and do not have to import Bugnsag from 'bugnsag'. It models the usage to Bugsnag.notifyException(exception[, name])

// app/utils/bugsnag-exception.js
export default function bugsnagNotifyException(e, name) {
  const onerror = Ember.onerror || function() {};
  e.name = name || e.name;
  onerror(e);
};

// app/components/some-component.js
import bugsnagNotifyException from '../utils/bugsnag-exception';
try {
  // Some code which might throw an exception
} catch (exception) {
  bugsnagNotifyException(exception, 'customErrorName');
}

I think ember-cli-bugsnag should expose more functionality for reporting error handling, but this is a good workaround that works for my use case.

[ghost]

Hey Alex. Really glad you were able to come to a solution in the end.

It's definitely not what that interface was originally designed for but I'm glad it works.

Are you happy for me to close the issue? I'm looking back to hearing from @cibernox on the specific motivation for the other PR.

I think ember-cli-bugsnag should expose more functionality for reporting error handling

Any chance you could give a concrete example? I often find myself being tempted to implement features that wrap around features that bugnsnag-js implements directly and I'm relatively convinced that the right thing there is to let the user work that out for themselves. E.g. see #53.

[flexyford]

Woops, closed this issue by merging my branch, did not know github did that when referencing different repos? Yes I'm fine closing this issue, but will let @binhums decide

I often find myself being tempted to implement features that wrap around features that bugnsnag-js implements directly and I'm relatively convinced that the right thing there is to let the user work that out for themselves.

IMO a bugsnag wrapper should be exposed so that ember-cli-bugsnag users can utilize all the great things bugsnag-js has to offer without doubling up on the bugsnag-js dependency (Once for ember-cli-bugsnag and once again for the bugsnag methods which are not exposed by the addon).

Regardless, I think the README could benefit from an explanation of "reporting handled exceptions"

Reporting Handled Exceptions

To handle exceptions but still log them in Bugnag we have to call Ember.onerror directly:

try {
  // Some code which might throw an exception
} catch (exception) {
  exception.name = 'customErrorName';    // Default Name: 'Error'
  if (Ember.onerror) { Ember.onerror(e) };
}

[ghost]

@flexyford sorry for letting things hang so long, it didn't feel so actionable at the time.

exposed so that ember-cli-bugsnag users can utilize all the great things bugsnag-js has to offer without doubling up on the bugsnag-js dependency

Agreed

Regardless, I think the README could benefit from an explanation of "reporting handled exceptions"

Agreed, I know Bugsnag is quite limited on what you can do here. One thing that I'm not sure on is the distinction between marking a project as a JS project vs an Ember.js project. Perhaps they do a better job of grouping errors that are specific to the Ember.js ecosystem.

if (Ember.onerror) { Ember.onerror(e) };

It probably makes more sense to rethrow a wrapped exception. I wouldn't want to make any assumptions about the consuming app's setup. Otherwise looks great. Can you add a PR for the docs?

Sorry I've let this sit open a while. @flexyford correct me if I'm wrong but the two actions are:

• Add README docs for renaming exceptions and
• Get clarification from @cibernox on excluding in test instances.

[machty]

I agree with @flexyford's (at least initial) assessment that we should NOT prevent the bugsnag library from being included, but should rather than the notifyReleaseStages config determine which bugsnags should actually be reported.

I talked with @cibernox about this; it would be bad if we suddenly started importing Bugsnag if people aren't prepared for it, since it might mean they'll start getting real bugsnags from test environments if they haven't spent time tweaking their notifyReleaseStages config. Perhaps the best way to go is to make this configurable?

[cibernox]

@machty I think that a config option that overrides the default behavior and allows people to opt in to inporting bugsnag in test environment makes sense, it would be backwards-compatible and easy to implement.

[samajammin]

Was able to resolve my issue thanks to this thread - cheers!

+1 to ember-cli-bugsnag exposing the ability to report handled exceptions to bugsnag. I think that would be a great feature. Not sure the best way to implement... perhaps exposing a bugsnag service that could be injected by routes & components?

For context on my use case, I have a bunch of components that .catch() promise exceptions in order to display the error to the user via toast notification. When this happens, I still want to notify bugsnag of the error.