Allowing rejected promises in tests

[marcoow]

I'm writing tests for Ember Simple Auth and want to test the case that the authenticator rejects authentication. Therefor I stub its authenticate method to return a rejected promise:

sinon.stub(authenticator, 'authenticate').returns(Ember.RSVP.reject('error'));

The problem now ist that this immediately causes RSVP.onerrorDefault to run which fails the test: Test.adapter.exception(error);.

I don't see a clean way to disable that behavior so that it allows this specific promise rejection. A working solution I came up with is

var originalPromiseError;

beforeEach(function() {
  originalPromiseError = Ember.RSVP.Promise.prototype._onerror;
  Ember.RSVP.Promise.prototype._onerror = Ember.K;
  sinon.stub(authenticator, 'authenticate').returns(Ember.RSVP.reject('error'));
});

afterEach(function() {
  Ember.RSVP.Promise.prototype._onerror = originalPromiseError;
});

but that's obviously not sth. I want to be doing as it uses private API.

I think it would be great to have some way of using unhandled promise rejections in tests. While that would most likely not be of use for applications, I assume that more addons than just ESA might have a need for sth. like this.

[marcoow]

This only happens when I do the stubbing in beforeEach (I'm using mocha), not in an it. I assume that that works as the rejection is handled inside of the it but not in the beforeEach?

[stefanpenner]

seems dubious to allow rejections during tests

[marcoow]

Yeah, understand that. But isn't the use case actually valid?

[hwhelchel]

I believe I'm running up against this in my acceptance tests for ESA and ESA devise. What I have is

describe('with invalid login', function() {
  beforeEach(function() {
    this.options.status = 401;

    TestHelper.stubEndpointForHttpRequest('/accounts/sign_in', { error: 'invalid_grant' }, this.options);
    visit('/accounts/sign_in');
    fillIn('#email', this.account.email);
    fillIn('#password', this.account.password);
    click('input[type=submit]'); // blows up here when authenticate promise rejects
  });

  it('should not validate the session', function() {
    andThen(() => {
      expect(currentSession().isAuthenticated).to.equal(false);
      expect(currentSession().get('content.secure')).not.to.include.keys(...Object.keys(this.resp));
    });
  });
});

Is there an alternative way of testing this user flow?

[jamesarosen]

@stefanpenner

seems dubious to allow rejections during tests

Seems perfectly normal to me. How else would I test that I show an appropriate error message when I get a 422 response from some API?

Also: this behavior is not just bad, it's also inconsistent.

This doesn't break the test:

new Ember.RSVP.Promise(function(resolve, reject) {
  reject();
});

But this does:

new Ember.RSVP.Promise(function(resolve, reject) {
  reject("anything");
});

because in the first case Test.adapter.exception(error); is Test.adapter.exception(undefined); and the test adapter ignores it.

[jamesarosen]

Note that this is intimately related to ember-cli/rfcs#19, which proposes switching ember-cli's default AJAX adapter from ic-ajax to a fetch wrapper. Unlike jQuery.ajax, fetch (per spec), resolves successfully with an error object for 4xx and 5xx.

It may be the case that the long-term path for Ember is don't use reject and .catch(fn) for control flow. But that's not what apps expect right now.

[stefanpenner]

@jamesarosen I should have clarified. Unhandled Rejections with error objects are dubious to allow.

Handle rejections or unhandled without a reason that is instanceof error. Are fine, and should already not be asserted

[jamesarosen]

Somehow I think you and I are heading towards the same conclusion, yet I don't quite understand what you said ;)

Which of the following are OK in your mind?

// reject with undefined, no catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject();
});

// reject with undefined, catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject();
}).catch(Ember.K);

// reject with string, no catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject("tsk tsk!");
});

// reject with string, catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject("tsk tsk!");
}).catch(Ember.K);

// reject with object, no catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject({ any: 'old', object: 'literal' });
});

// reject with object, catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject({ any: 'old', object: 'literal' });
}).catch(Ember.K);

// reject with Error, no catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject(new Error("tsk tsk!"));
});

// reject with Error, catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject(new Error("tsk tsk!"));
}).catch(Ember.K);

[stefanpenner]

All except for the unhandled rejection that rejects with the error. ( sorry on mobile or I would have cited the one)

[jamesarosen]

All except for the unhandled rejection that rejects with the error

This I can get on board with :)

[stefanpenner]

@jamesarosen yup I thought so. And that is also the current behavior. It also handles more rejection scenarios. As long as they are handled in the same turn of the actions queue flush they will be good to go.

I can likely expand that further yet, as we recently added some extra hooks to the run-loop.

Anyways, I believe the current state is actually what one wants.

If what I described does not happen, we must consider it a regression and fix .

[twokul]

@marcoow I think you can work around it by Ember.Test.adapter = null; in beforeEach or before (depends if it's qunit or mocha)

[jamesarosen]

And that is also the current behavior.

Then why is the following code breaking?

function failToDoX() {
  return new Ember.RSVP.Promise(function(resolve, reject) {
    Ember.run(null, reject, { an: 'error' });
  });
}

test('it catches', function(assert) {
  var caught = false;

  failToDoX()
  .catch(function(e) { caught = e; });

  assert.ok(caught != null);
});

I get to the catch, but the test still fails. Is it because failToDoX does its own run-loop wrapping?

[twokul]

@jamesarosen I might be wrong but I think it depends on how Ember.Test.adapter is setup up.

RSVP. onerrorDefault will check if there's an adapter set up and depending on the result either throw an error (which causes catch to be called w/o side effects) or it will print an error in the console and add an ok(false) assertion.

So the test above should work in isolation but it might not work when running whole test suite with acceptance/integration/unit tests.

[jamesarosen]

@twokul yeah, but that doesn't jibe with what @stefanpenner is saying:

All except for the unhandled rejection that rejects with the error [should be OK]

and

And that is also the current behavior. It also handles more rejection scenarios.

What worries me is

As long as they are handled in the same turn of the actions queue flush they will be good to go.

That suggests to me that if something does its own run-loop-wrapping, I can't .catch to prevent the promise failure from bubbling.

[stefanpenner]

Ember.run(null, reject, { an: 'error' });

2 things:

the run forces the flush early that being said, I'm not sure why a non-error object is causing the assertion to throw, that appears like a bug.

[stefanpenner]

these lines dont quite look correct https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/ext/rsvp.js#L60-L67

[stefanpenner]

Ultimately if something doesn't jive with what i said, it is very likely a bug/omission/regression

[stefanpenner]

@marcoow I think you can work around it by Ember.Test.adapter = null; in beforeEach or before (depends if it's qunit or mocha)

this really isn't advised, be aware of the 👣 🔫

[marcoow]

@stefanpenner: yeah, that might hide the problem but of course isn't actually a solution.

IIRC, I ran into this: https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/ext/rsvp.js#L61 and that actually makes the test fail regardless of whether the rejection is handled in the test or not.

[stefanpenner]

I have enumerated an added tests to the current behavior: https://github.com/emberjs/ember.js/pull/11903/files

[stefanpenner]

@marcoow https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/ext/rsvp.js#L61 should be fixed with #11903 Turns out we were using a quite old version of RSVP and some more recent refactoring scenario solved the problem. This should correctly align it with my above statements, and also add the appropriate test cases.

https://github.com/tildeio/rsvp.js/blob/master/lib/rsvp/promise.js#L177-L181

[marcoow]

@stefanpenner great, looks good!

[jamesarosen]

All of a sudden I'm seeing this on Ember v1.13.11, which uses RSVP v3.0.14.

The relevant code looks like

function someMethod() {
  if (someCondition) {
    return RSVP.reject({ type: 'error', body: 'someCondition failed' });
  }
}

and the relevant test:

test('when someCondition is false', function(assert) {
  return theObject.someMethod()
  .catch((error) => {
    assert.equal(error.type, 'error', 'rejects with an error');
  })
});

When I run the test in isolation, it passes. When I run the full test suite, I get

not ok 778 PhantomJS 2.1 - Integration: ApiKeys: apiKeys.load rejects when sudo is not enabled
    ---
        actual: >
            false
        expected: >
            true
        stack: >
            http://localhost:7357/assets/test-support.js:3879:17
            ...
        message: >
            {type: error, body: someCondition failed}

[stefanpenner]

@jamesarosen that doesn't seem correct and the catch handler is attached sync. Can you provide a quick reproduction?

[jamesarosen]

and the catch handler is attached sync

Oh! Maybe it's coming from an earlier test. Like one I'm not attaching a catch handler to because I expect it to pass.

[stefanpenner]

@jamesarosen I would consider that test helper an anti-pattern, but the solution would be to simply handle the result of the callback before returning to run itself. So it is considered handled before the turn completes.

[jamesarosen]

I would consider that test helper an anti-pattern

I have no doubt you would. But the only other option is to wrap each test in Ember.run(() => {...}) since the thing that catches the rejection is the test. It's not some other client in the app. The test is asserting that the method returned a rejection. And I need the run because there are set calls. Without it, I get the "You turned on testing mode, which disabled the auto-run."

If I seem frustrated, it's because I am. I appreciate your work and your help on this issue, but everything I suggest gets dismissed as "not what the core team wants you to do."

I'm absolutely open to other ideas, though! If you have a suggestion on how to test -- at an integration or unit level -- that a method returns a rejected promise under certain conditions and has set behavior under those conditions, I'm all ears!

[stefanpenner]

"not what the core team wants you to do."

This often correlates to, don't cause yourself extra pain/frustration. The suggestions are not arbitrary.

I'm absolutely open to other ideas, though! If you have a suggestion on how to test -- at an integration or unit level -- that a method returns a rejected promise under certain conditions and has set behavior under those conditions, I'm all ears!

can you provide a jsbin, or a demo app with the problem and i will gladly do so.

[jamesarosen]

Thanks, @stefanpenner. I'll keep digging and see what I can turn up.

[jamesarosen]

@stefanpenner pointed me in the right direction. Ember.RSVP will automatically enqueue run-loops without Ember.run and it won't trigger the auto-run error in test mode. That means I can just do

test('foo', function(assert) {
  return thing.signIn('username', 'bad password')
  .catch((err) => {
    assert.equal(err.body, 'Invalid username or password');
  });
}):

and it works just fine :)

[stefanpenner]

@jamesarosen we have been slowly massaging all these to continue to remove traps/pain points, its very possible your recall a previous state where this was not handled transparently. I suspect you will continue to see improvement in this domain.

[jamesarosen]

Given that ember-data v1.x calls Ember.run(null, reject, someError), do you have any suggestions on how to best turn off these erroneous test failures? Is it as simple as overriding ajax to not wrap in run?

[toranb]

I'm not here to debate the "right or wrong" of testing exceptions but I did want to offer a workaround for those like me who value testing 500 errors in acceptance tests (whether you are using ember-data or something else that wraps RSVP). Full credit for this goes to my good friend @williamsbdev who found the least hacky/but most functional way to get this working :)

var application, originalLoggerError, originalTestAdapterException;

module('Acceptance to tdd error handling in your ember app', {
    beforeEach() {
        application = startApp();
        originalLoggerError = Ember.Logger.error;
        originalTestAdapterException = Ember.Test.adapter.exception;
        Ember.Logger.error = function() {};
        Ember.Test.adapter.exception = function() {};
    },
    afterEach() {
        Ember.Logger.error = originalLoggerError;
        Ember.Test.adapter.exception = originalTestAdapterException;
        Ember.run(application, 'destroy');
    }
});

test('some test that blows up when I try to tdd and a 500 makes RSVP reject', (assert) => {
    server.post('/api/foo/1', (db, request) => {
        //return a 500 with ember-cli-mirage/or whatever ajax stub library you choose
    });
    visit('/foo');
    fillIn('#my-input', 999);
    click('#my-btn');
    andThen(() => {
        //assert your modal shows up/or whatever biz thing happened
    });
});

[williamsbdev]

@toranb - Thank you for calling attention to this.

I would like to give some background. One of the applications that we were working on had great validations before the request was made so it would not get a 400. Instead of handling all the errors with each ajax request, we wanted to catch all the errors across the application. So we setup something like this:

Ember.RSVP.on("error", function() {
    // handle 401
    // handle 403
    // handle 5XX errors
    // handle other errors
});

The problem then came when we wanted to test. This is why the monkey patching (done above in the comment by @toranb) on Ember.Logger.error and Ember.Test.adapter.exception were done. I was not terribly fond of what I did but it gave me some confidence that the application would respond in the desired manner.

I completely understand what @stefanpenner was saying about handling the error by the end of the run loop. When running the tests, you don't know if the error has been handled and the test framework is going to let the user know that something is broken. This is great. We just took advantage of the Ember.RSVP.on("error" which allowed it to handle the error and had to trick the test into allowing it.

[sukima]

FYI this is especially troublesome when attempting to use async/await The wrapping in a run loop is not something you can accomplish with async functions because run is not asynchronous. You can not express this using async/await, co-routines (like ember-co) or ember-concurrency tasks in a unit test:

test('it really has come to this', function (assert) {
  return run(() => {
    return RSVP.then(() => {
      assert.ok(false, 'expected promise to be rejected');
    }).catch(error => {
      assert.ok(true, 'expected promise to be rejected');
    });
  });
});

I challenge anyone who can show how to express a try { await ... } catch { } inside an Ember.run(). Good luck!

It is sad this is the state of affairs. If only there was a way to pause a run loop do you thing and the resume the run loop.

[dmuneras]

I think this post from EmberMap could be very helpful for people that end up in this conversation:

https://embermap.com/notes/86-testing-mirage-errors-in-your-routes

[lolmaus]

@dmuneras The article by @samselikoff describes my case precisely:

GIVEN that my server will error in response to XYZ network request
WHEN my app makes a request to XYZ
THEN I expect my app to behave in this way

Unfortunately, the article only covers suppressing console logging. It does not address the original issue: preventing an acceptance test from failing when an Ember Data request rejects and is unhandled.

We intercept the error with a decorator that shows a flash message to the user. But we re-throw the error, so that it's caught by an error reporter such as Sentry.

The solution outlined above, overriding Ember.Test.adapter.exception, does not work because of this:

// Ember 2.17 and higher do not require the test adapter to have an `exception`
// method When `exception` is not present, the unhandled rejection is
// automatically re-thrown and will therefore hit QUnit's own global error
// handler (therefore appropriately causing test failure)

https://github.com/emberjs/ember-qunit/blob/5cb60f696c24667f0cc2ce73e575d9f2b1d1a62c/addon-test-support/ember-qunit/adapter.js#L60-L75

Any ideas?

PS Yes, I know that we could have our flash message decorator report to Sentry directly, so that we don't need to re-throw the error. But that's not its responsibility! And I don't want to sprinkle the app with Sentry reportings, instead of having it intercept all errors globally.

[amk221]

@lolmaus For that exact scenario I do this:

// Inside a test
this.errorHandlingService.squelch(error => {
  return error instanceof InternalServerError;
});

...where the error handling service looks like this

[lolmaus]

@amk221 So you're overriding Ember.onerror. This doesn't work for me because Ember.onerror is undefined. @rwjblue said it's "a user provideable hook (not something that Ember itself provides)."

You're also doing RSVP.on(), but I believe that would register an event handler without overriding existing handlers.

[amk221]

@lolmaus

Ember.onerror = ...
RSVP.on('error', ...);

...has been what the guides tell you to do for years. That, in combination with my previous link, gives you full control over errors in the app and test suite. I've never had any issues. Sorry I couldn't help!

[lolmaus]

@amk221 That gives full control over reacting to errors. It does not stop QUnit from failing the test when there's an unhandled Ember Data network error.

[amk221]

It does though? I have this top level error handling service in an addon and a test suite for it

[amk221]

@lolmaus I've just realised the reason this works for me, is because my AJAX requests are routed through RSVP (because I use ember-fetch), hence the service can squelch it. If you use native fetch, then it won't work.

Here is a test proving it is possible to test error states easily. It is something I wanted for many years as you can see from this old issue, which is pretty much a duplicate of what you want.

[lolmaus]

It does though?

I do this:

  debugger;

  Ember.onerror = (error) => {
    debugger;
  };

I can see the first debugger statement firing, but the second one never fires, and tests are red.

Anyway, I believe, Ember.onerror is (was?) a hook that lets you log your errors to services like Sentry/Bugsnag. It's not a way to intercept errors.

---

this works for me, is because my AJAX requests are routed through RSVP (because I use ember-fetch), if yours just use fetch, then the squelching won't work.

We use Ember Data, currently at version 3.13.1. No jQuery.

---

I'm a bit at a loss here. The previous solution of using Ember.Test.adapter.exception has been removed from Ember, and there is no replacement.

[lolmaus]

Reported to ember-qunit: emberjs/ember-qunit#592

[lolmaus]

Here's what I figured out:

1. QUnit is designed to fail tests on all errors, including unhandled rejected promises. Mocha won't do that unless you use an assertion against a rejected promise.

2. QUnit does not have an API to squelch those errors, making a test pass. There is assert.throws, but:

   • It can't be used with Cucumber-style tests, which execute a linear list of steps, and steps can't be nested.
   • It seems to be synchronous. If I get it right, you can't use it with async code and instead have to catch the failing promise.
   • But there is no access to Ember Data promises from an acceptance test.

3. Ember.onerror is an old and quite poorly documented feature of Ember:

   • It is mentioned once in the guides as a simple hook for Sentry/Bugsnag.
   • It's never mentioned in the API docs.
   • It does not have an export.
   • It does not offer pub/sub, meaning two addons can't use it.
   • ember-mocha does not seem to use it.
   • ember-qunit does use it (see below), but does not document it, keeping us clueless.

4. I had an impression that Ember.onerror is a candidate for deprecation and should be avoided. @amk221 kindly helped me understand that it has special behavior in ember-qunit: catching an error in Ember.onerror would prevent QUnit from failing a test!

5. At the same time, QUnit would ensure Ember.onerror is not catching all errors. In case it does, you should see a failing test case. To avoid this, you need to re-throw errors after catching them in Ember.onerror.

6. @amk221 pointed out that Ember.onerror only catches errors that are thrown within the Ember runloop. For some reason, this throw:

   async someMethod() {
     try {
       await store.query(/*...*/);
     } catch (e) {
       throw e;
     }
   }

   ...would throw the error outside of the runloop, and Ember.onerror will ignore it!

   The solution is to wrap it with run from '@ember/runloop'. This is the first time I've had to use run in app code.

Now I have control over errors in tests again, which I lost after Ember.Test.adapter.exception was removed.

Big thanks to @amk221 for taking time and effort to help me figure this out.

[adong]

Later to the party. I found something interesting, and think the reason it fails is that qunit has some changes

Qunit:

CLI: Ensure an unhandled rejection results in a failed test.
https://github.com/qunitjs/qunit/blob/master/History.md#250--2018-01-09

It's purely a testing thing that the errors are dispatched with runloop, even though you can catch in your test, qunit will pause your tests.

Similarly, once this error happened

  test('its gonna fail', async function(assert) {
    try {
      setTimeout(() => { 
        //console.log('test'); 
        throw new Error('02/10/2020');
      }, 1000);
    } catch (err) {
      console.log('err here', err);
    }
  });

Output screenshot:

You can see above example will only fail the test cases, not block the rest of tests.

But, the following example will pause the tests

  test('its gonna fail and blocks', async function(assert) {
    try {
      const result = await new Promise((resolve, reject) => {
          setTimeout(() => {
            throw (new Error("error"))
          }, 100)
      });
    } catch (err) {
      console.log('err here', err);
    }
  });

The error is caused in like others might mentioned in previous comments:
from RSVP.js

RSVP.on('error', onerrorDefault);

Which does pretty much the latter case.

So, what's one of the solutions? I don't know. I end up writting a catch statement in my business logic.

See reference: Stackoverflow: Try... Catch in async await not catching error