visit() throws TransitionAborted error

[simonihmig]

As discussed in Slack, I was unexpectedly seeing failing tests where a transition was aborted.
ApplicationInstance#visit is throwing it here

As I said I cannot share the app, but this is the human readable test spec (we have a rather exotic test setup with ember-cli-yadda 😉):

  Scenario: Access restricted page as invited user and login
    Given there exists a user with username "testname" and password "testpw"
    And I am an invited user
    And I am on the homepage
    When I go to the user profile page
    Then I should still be on the homepage
    And I should see the login modal
    When I insert "testname" as the username
    And I insert "testpw" as the password
    And I submit the login form
    Then I should be on the user profile page

The I go to the user profile page step is throwing the exception, which is calling visit('/user/profile') internally.

I quickly assembled this little repro, that resembles what we were doing in a very minimal way:
https://github.com/simonihmig/ember-visit-transition-abort

All the acceptance tests are passing, with the exception of this last one:
https://github.com/simonihmig/ember-visit-transition-abort/blob/master/tests/acceptance/auth-test.js#L36-L40

[simonihmig]

@rwjblue did you have a chance to look at this?

I am happy to work on a PR, if we agree what the solution should be. (this is the only remaining issue left for our app's test suite refactoring PR ;))

I am not fully aware of the inner workings, but it puzzles me that clicking on the link-to is different to using visit. (See this vs. that)

[spencer516]

@simonihmig — we've been running into this too. Just opened a PR with a failing test and a place to start: #373

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

[esbanarango]

☝️ We're having the same issue when migrating to the new testing API.

[NullVoxPopuli]

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);
}

[ppcano]

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;
    }
  }

};

[NullVoxPopuli]

@ppcano that's pretty much what I'm doing now (for re-usability)

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

async function visit(url: string) {
  try {
    await dangerousVisit(url);
  } catch (e) {
    console.error(e);
  }
}

// tests omitted

I like your explicit checking for the TransitionAborted though

[tpetrone]

Also experiencing this when return transitionTo('_routeName_') inside beforeModel() hook.
New test suite. Ember 3.4.1

[chrisseto]

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();
}

[velrest]

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.

[rwjblue]

What is in your beforeModel? Is it transitioning?

[netes]

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.

Make sure you're returning something from an asynchronous beforeModel, not just await.

[NullVoxPopuli]

what would you return?

I regularly do this:

async beforeModel() {
  if (await some condition not met) {
    return this.transitionTo('do something');
  }
}

[velrest]

Im returning nothing:

  async beforeModel() {
    if (logic) {
      await something;
    }
    if (!logic2) {
      this.replaceWith("/some/url");
    }
  }

Should i put a return before this.replaceWith? If yes, can someone explain to me why?

[rwjblue]

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

[velrest]

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

[velrest]

Sadly i cant really debug it since the stack trace is just backburner calls. And stepping through the this.replaceWith was not as helpful as i thought.

[James1x0]

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

[sethphillips]

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]

Someone want to throw together a small repro repo, might be worth stepping through this again 🤔

[sethphillips]

Thanks for the fast response Robert.
https://github.com/sethphillips/transition-aborted-reproduction
great route works find and tests pass
borked route fails with TransitionAborted

[rwjblue]

Awesome, I'll try to poke at the repro this week. I remember looking through @simonihmig's initial repro, but things have changed enough upstream that it seems worth digging through again...

[patsy-issa]

Fwiw using redirect() to redirect instead of before/after/model does not cause visit to fail.

[vfuto]

What if you invoke transitionTo from router service? this.transitionTo is deprecated.

@service router;
...
   this.router.transitionTo(route)
...

https://deprecations.emberjs.com/v3.x/#toc_routing-transition-methods

[NullVoxPopuli]

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? 🙃

[angelayanpan]

is this resolved?

[NullVoxPopuli]

nope.

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

[angelayanpan]

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

[elwayman02]

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

You're welcome 😂

[gnclmorais]

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?

[davideferre]

Is there any updates on this issue? In a test the only way to succede is to catch the exception generated by the visit() statement and check if the error message is TransitionAborted. It's a tricky way of testing and it can be misleading.
I also agree the idea by @gnclmorais

[fastfedora]

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.

[barryofguilder]

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.

[NullVoxPopuli]

If someone wants to do the Code/PR work, I can advocate for it to the ember core team and write up an RFC if needed, etc

[davidtaylorhq]

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: tildeio/router.js#335

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