Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transition.abort() causes 500 responses #202

Open
arjansingh opened this issue May 27, 2016 · 11 comments
Open

transition.abort() causes 500 responses #202

arjansingh opened this issue May 27, 2016 · 11 comments

Comments

@arjansingh
Copy link
Contributor

As @rwjblue pointed out in #195, aborting a transition in FastBoot causes a TransitionAborted error to be thrown by the server, resulting in the request failing and the response returning a 500 error.

Here is an example stack trace:

Error: TransitionAborted at handleReject (/code/ember-bugs/dist/fastboot/vendor.js:14132:17)
 at tryCatch (/code/ember-bugs/dist/fastboot/vendor.js:63911:14)
 at invokeCallback (/code/ember-bugs/dist/fastboot/vendor.js:63926:15)
 at publish (/code/ember-bugs/dist/fastboot/vendor.js:63894:9)
 at publishRejection (/code/ember-bugs/dist/fastboot/vendor.js:63829:5)
 at /code/ember-bugs/dist/fastboot/vendor.js:42159:7 at Queue.invokeWithOnError (/code/ember-bugs/dist/fastboot/vendor.js:10447:18)
 at Object.Queue.flush (/code/ember-bugs/dist/fastboot/vendor.js:10502:11)
 at Object.DeferredActionQueues.flush (/code/ember-bugs/dist/fastboot/vendor.js:10310:17)
 at Object.Backburner.end (/code/ember-bugs/dist/fastboot/vendor.js:10665:25)
 at [object Object]._onTimeout (/code/ember-bugs/dist/fastboot/vendor.js:11231:18)
 at Timer.listOnTimeout (timers.js:92:15)

I'll try to look at this when I get some time. If anyone has any thoughts on what might be causing this, please chime in.

@jasonmit
Copy link
Contributor

jasonmit commented Jun 21, 2016

It believe it's happening here: https://github.com/ember-fastboot/fastboot-express-middleware/blob/master/index.js#L54-L65

Similar to the handling of UnrecognizedURLError, we might want to also call next on TransitionAborted

jasonmit added a commit to jasonmit/fastboot-express-middleware that referenced this issue Jun 21, 2016
@jasonmit
Copy link
Contributor

Actually, I'm unsure how we should handle aborted transitions so I closed my related PR. What is the behavior that you are expecting?

@tomdale
Copy link
Contributor

tomdale commented Jun 24, 2016

@arjansingh Ping when you get a chance

@arjansingh
Copy link
Contributor Author

In my opinion the expected behavior on an abort should be to just continue execution of the code.

Aborts are usually followed with a transitionTo, which would result in 307 redirect: https://github.com/ember-fastboot/ember-cli-fastboot/blob/master/app/locations/none.js#L29-L49

If there is no transition then the expected behavior from what I can see is to just render whatever route we are on with the information we have available, like we would for a non-fastbooted page.

Here's the code for abort() for reference: https://github.com/tildeio/router.js/blob/master/lib/router/transition.js#L184-L200

@rwjblue
Copy link
Member

rwjblue commented Jun 25, 2016

Confirm. I do not think an aborted transition should be directly considered an error condition.

@Gaurav0
Copy link
Contributor

Gaurav0 commented Dec 7, 2016

@rwjblue Reproduction without explicit transition.abort: https://github.com/Gaurav0/fastboot-issue-202.git

@rwjblue
Copy link
Member

rwjblue commented Jan 4, 2017

I did a bit of investigation into this today (thanks to @Gaurav0 reproduction), and have isolated the issue.

This error is currently (speaking about Ember 2.10+) only occurring when a .transitionTo / .redirectTo is being done asynchronously in afterModel. Doing the redirect in the redirect or model hooks work without an error.

In afterModel the following would work without error:

afterModel() {
  this.transitionTo('something');
}

And this would not:

afterModel() {
  return Ember.RSVP.Promise.resolve()
    .then(() => {
      this.transitionTo('something');
    });
}

The reason for this, is that we currently only allow a transition to be aborted while it is underway (and we use this.get('router').router.activeTransition to track that here), but once afterModel has ran we immediately clear the activeTransition (see here).

@rwjblue
Copy link
Member

rwjblue commented Jan 4, 2017

I commented on ember-learn/ember-api-docs#128 with a more concrete solution over there.

xg-wang pushed a commit to xg-wang/ember-cli-fastboot that referenced this issue Nov 16, 2020
tobiemh added a commit to surrealdb/ascua that referenced this issue Jan 20, 2021
When running in Fastboot, we can’t abort a transition inside an asynchronous promise. See ember-fastboot/ember-cli-fastboot#202. Instead we need to use the route’s redirect hook, and make use of replaceWith instead of transitionTo, so that the back button still works correctly.
tobiemh added a commit to surrealdb/ascua that referenced this issue Jan 20, 2021
When running in Fastboot,aborting a transition inside an asynchronous promise, still causes an error when in the redirect hook, unlike is stated here: ember-fastboot/ember-cli-fastboot#202. Instead we will not wait for any Surreal connection attempt, instead relying on the user to use the @attempted decorator in the application route.
@rtablada
Copy link

rtablada commented Aug 8, 2022

Ran into this today still with fastboot.

Was working on an application started using Engage and went to setup redirects to a login page and hit the same error.

I went to recreate the error in a fresh Ember app thinking there were still underlying router.js errors and missing catch assertions in Ember, but the error now seems to be isolated to applications using fastboot.

image

https://github.com/flightplanjs/example-app-2


Some of the above "solutions" and OSS commits skip redirects with fastboot, but this is a bad solution since it would then cause the application to only work in rehydration mode, and would require full boot and client side paint along with potential state leaks of protected routes.

@alexraputa
Copy link

@rtablada, Can u try:

async beforeModel(transition: Transition) {
  transition = this.router.transitionTo('login'); // or this.router.replaceWith('login');

  return await super.beforeModel(transition);
}

@rtablada
Copy link

rtablada commented Aug 8, 2022

@alexraputa still has the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants