ember-test-helpers: visit() throws TransitionAborted error

Just to check in here, I’ve been trying to track down exactly what is going on and I think I have a fairly good picture. Essentially what is happening is that the secondary transition completes during the beforeModel/model/afterModel of the first one, and since the full secondary transition has completed the router’s error handling throws this error instead of entangling the new transition with it like we do when there is an active transition.

Ultimately, returning anything other than the actual resulting Transition object will cause the error reported here to bubble up. This includes making any of the beforeModel/model/afterModel methods themselves async (because then you are returning a Promise not a Transition), or returning an RSVP.Promise that later resolves to a Transition (e.g. return this.store.findRecord('post', 1).then((record) => if (record.whatever) { return this.transitionTo(...); }).

I know this is what folks were reporting all along, but now I actually understand it 😩. Sorry for taking so long to dig in here. I’m trying to work up a solution using @sethphillips’s reproduction.

I am also experiencing this issue when migrating our test suite (Ember 2.17 & ember-cli-qunit ^4.1.1)

We are using transition.abort and transition.retry to handle route authentication at the route level as suggested on the Preventing and Retrying Transitions guide.

Based on @NullVoxPopuli guidance, our current workaround has been to implement a wrapper of the visit helper:

import { visit } from '@ember/test-helpers';

export default async function visitWithAbortedTransition(url) {

  try {
    await visit(url);
  } catch(error) {
    const { message } = error;
    if (message !== 'TransitionAborted') {
      throw error;
    }
  }

};

Minor addition to @ppcano’s solution as the original settle is interrupted by the error.

import { settled, visit as _visit } from '@ember/test-helpers';

export async function visit(url: string) {
    try {
        await _visit(url);
    } catch (e) {
        if (e.message !== 'TransitionAborted') {
            throw e;
        }
    }

    await settled();
}

do we have a path forward? or a list of what this is waiting on?

do we have a resolution here?

having visit throw on transition isn’t good DX, imo.

I know fixing it would have to be a breaking release… but… that’s fine? 🙃

It looks like this issue extends beyond the visit() helper. It affects any use of transition.followRedirects() (which is essentially what visit() is doing).

I have a failing test case and fix for router.js here: https://github.com/tildeio/router.js/pull/335

And then a tweak to Ember core so that it leans on followRedirects() instead of reimplementing the same logic: https://github.com/emberjs/ember.js/pull/20574

this Issue is mentioned in 20 different places in our code. with ToDos

@simonihmig — we’ve been running into this too. Just opened a PR with a failing test and a place to start: https://github.com/emberjs/ember-test-helpers/pull/373

(I’d be curious if it fixes your issue)

I discovered a new workaround after not being able to get the other workarounds mentioned above working in Ember 4.

I’m not entirely sure why this is working, but doing the transition within a function delayed via setTimeout fixes the error for me:

setTimeout(() => router.transitionTo(route), 0);

I think this is because it moves the entire transition out of the promise chain, so the rejection is not captured by ember-test-waiters. So this is still likely a hack and a workaround, rather than a permanent solution. But when I do this, the TransitionAborted rejection no longer appears (nor the subsequent testWaiter.endAsync called with no preceding testWaiter.beginAsync call. which appears in the console after the TransitionAborted rejection).

For context, the transition itself is happening within ember-simple-auth, which I’ve overridden to make the hack work. Currently we have an authenticated route that looks like:

export default class AuthenticatedRoute extends Route {
  @service session;
  ...

  async beforeModel(transition) {
    const loggedIn = this.session.requireAuthentication(transition, 'login');

    if (!loggedIn) {
      return;
    }

    ...
  }
}

The requireAuthentication eventually calls transitionTo when the user is not logged in, which is what generates the error. To fix this, I overrode requireAuthentication within the session service:

export default class AppSessionService extends SessionService {
  requireAuthentication(transition, routeOrCallback) {
    if (ENV.environment !== 'test' || typeof routeOrCallback !== 'string') {
      return super.requireAuthentication(transition, routeOrCallback);
    }

    const owner = getOwner(this);
    const router = owner.lookup('service:router') ?? owner.lookup('router:main');

    return super.requireAuthentication(
      transition,
      () => setTimeout(() => router.transitionTo(routeOrCallback), 0)
    );
  }
}

This works because the second parameter of requireAuthentication can be a route to redirect to or a callback to call when the user is not authenticated. We only ever pass a string route in, so wrapping it in a callback during testing to fix the error introduces minimal deviance from the production code.

@rwjblue Hmm in my failing test the problem occurs even with a return could there be another problem that im not aware of?

Im still having this issue, in my case this happens if i add a async to my beforeModel hook. After that, every test which renders this route throws a TransitionAborted.

I have some transitionTos happening in my afterModel hook – which is intentional – but in my test visit is raising a TransitionAborted exception.

The only way for my test to pass is:

try {
  await visit(url);
} catch(e) {
  console.error(e);
}

imo, someone should submit a PR that requires a breaking change of this library (major bump), and catches TransitionAborted errors. Then, a new helper should be added, unsafeVisit that does what today’s visit does

I would love to have this update so we don’t have to define our own to workaround this.

Riffing on @NullVoxPopuli, how about an extra key for the options the visit(…) method accepts? Something like { silenceTransitionAborted: true } or something like that… My thinking is that aborted transitions would not be the default and, with this extra option, it becomes explicit that we expect it. Thoughts?

@SebastianPetric I was able to workaround this issue by wrapping my visit calls in a try-catch block for tests that specifically intend to execute the aborted transition behavior. It’s not ideal, but it did temporarily unblock us.

We’re also banging our heads against this issue.

FWIW: The route seems to be loaded as expected after a non-async-visit()-then-timeout. However any subsequent assert() never fires:

visit(url);
await new Promise(resolve => setTimeout(resolve, 1000));    
console.log(currentURL() == url); // true
assert.equal(currentURL(), url); // never asserted, however the UI is fully loaded and interactive at pauseTest()

This issue makes acceptance testing unusable for us for the time being. I’d love to find a workaround, or even better be pointed towards fixing any possible bad-practice that is causing this. Or trust @rwjblue to find a patch/solution, of course.

Thanks!

I solved it by adding the error hook and using that to transition instead. Might be sufficient for some people.

just upgraded from 3.5.1 to 3.18.0 and am seeing this on any routes that we redirect in async beforeModel. similar to others, I’m doing something like this.

async beforeModel(){ let name = await fetch('athing') this.registration.setHostApp(name); return this.replaceWith('intended-route'); }

removing the async will remove the TransitionAborted error in tests.

@rwjblue We are running into this as well. The difference is that we are actually returning the abort. Any idea why this is still a problem based on the following code?

  async beforeModel () {
    const user = await this.auth.user,
          hints = user.get('shownHints');

    if (!hints || !hints.includes('documents')) {
      return this.transitionTo('account.documents.intro');
    }
  }

Previously user was a preset val, now it’s a promise, so we have to await. Hence the reason we are “running into it”.

Because otherwise, the beforeModel promise resolves before the replaceWith has resolved. This is commonly referred to as “floating promises” (@typescript-eslint/no-floating-promises), and will result in the AbortTransition rejection happening after the rest of the transition has moved further forward (and therefore will not be entangled with the router transition promise chain, which means the rejection happens out of band causing the reported error).