FEATURE: Improvement to history stack handling on server errors

The exception page is shown before Ember can actually figure out what the final destination URL we're going to is.
This means that the new page is not present in the history stack, so if we attempt to use the history stack to go back, we will actually navigate back by two steps.
By instead forcing a navigation to the current URL, we achieve the goal of going "back" with no history mucking.

Unfortunately, the actual URL that was attempted is not available. Additionally, this only works for the on-screen back button and not the browser back.

Additionally, several modernizations of the exception page code were made.

app/assets/javascripts/discourse/app/controllers/exception.js

@@ -1,5 +1,6 @@
 import { alias, equal, gte, none } from "@ember/object/computed";
 import discourseComputed, { on } from "discourse-common/utils/decorators";
+import DiscourseURL from "discourse/lib/url";
 import Controller from "@ember/controller";
 import I18n from "I18n";
 import { schedule } from "@ember/runloop";
@@ -31,15 +32,15 @@ export default Controller.extend({
   thrown: null,
   lastTransition: null,
 
-  @discourseComputed
-  isNetwork() {
+  @discourseComputed("thrown")
+  isNetwork(thrown) {
     // never made it on the wire
-    if (this.get("thrown.readyState") === 0) {
+    if (thrown && thrown.readyState === 0) {
       return true;
     }
 
     // timed out
-    if (this.get("thrown.jqTextStatus") === "timeout") {
+    if (thrown && thrown.jqTextStatus === "timeout") {
       return true;
     }
 
@@ -65,16 +66,18 @@ export default Controller.extend({
     this.set("loading", false);
   },
 
-  @discourseComputed("isNetwork", "isServer", "isUnknown")
-  reason() {
-    if (this.isNetwork) {
+  @discourseComputed("isNetwork", "thrown.status", "thrown")
+  reason(isNetwork, thrownStatus, thrown) {
+    if (isNetwork) {
       return I18n.t("errors.reasons.network");
-    } else if (this.isServer) {
+    } else if (thrownStatus >= 500) {
       return I18n.t("errors.reasons.server");
-    } else if (this.isNotFound) {
+    } else if (thrownStatus === 404) {
       return I18n.t("errors.reasons.not_found");
-    } else if (this.isForbidden) {
+    } else if (thrownStatus === 403) {
       return I18n.t("errors.reasons.forbidden");
+    } else if (thrown === null) {
+      return I18n.t("errors.reasons.unknown");
     } else {
       // TODO
       return I18n.t("errors.reasons.unknown");
@@ -83,30 +86,42 @@ export default Controller.extend({
 
   requestUrl: alias("thrown.requestedUrl"),
 
-  @discourseComputed("networkFixed", "isNetwork", "isServer", "isUnknown")
-  desc() {
-    if (this.networkFixed) {
+  @discourseComputed(
+    "networkFixed",
+    "isNetwork",
+    "thrown.status",
+    "thrown.statusText",
+    "thrown"
+  )
+  desc(networkFixed, isNetwork, thrownStatus, thrownStatusText, thrown) {
+    if (networkFixed) {
       return I18n.t("errors.desc.network_fixed");
-    } else if (this.isNetwork) {
+    } else if (isNetwork) {
       return I18n.t("errors.desc.network");
-    } else if (this.isNotFound) {
+    } else if (thrownStatus === 404) {
       return I18n.t("errors.desc.not_found");
-    } else if (this.isServer) {
+    } else if (thrownStatus === 403) {
+      return I18n.t("errors.desc.forbidden");
+    } else if (thrownStatus >= 500) {
       return I18n.t("errors.desc.server", {
-        status: this.get("thrown.status") + " " + this.get("thrown.statusText"),
+        status: thrownStatus + " " + thrownStatusText,
       });
+    } else if (thrown === null) {
+      return I18n.t("errors.desc.unknown");
     } else {
       // TODO
       return I18n.t("errors.desc.unknown");
     }
   },
 
-  @discourseComputed("networkFixed", "isNetwork", "isServer", "isUnknown")
-  enabledButtons() {
-    if (this.networkFixed) {
+  @discourseComputed("networkFixed", "isNetwork", "lastTransition")
+  enabledButtons(networkFixed, isNetwork, lastTransition) {
+    if (networkFixed) {
       return [ButtonLoadPage];
-    } else if (this.isNetwork) {
+    } else if (isNetwork) {
       return [ButtonBackDim, ButtonTryAgain];
+    } else if (!lastTransition) {
+      return [ButtonBackBright];
     } else {
       return [ButtonBackBright, ButtonTryAgain];
     }
@@ -114,14 +129,25 @@ export default Controller.extend({
 
   actions: {
     back() {
-      window.history.back();
+      // Strip off subfolder
+      const currentURL = DiscourseURL.router.location.getURL();
+      if (this.lastTransition && currentURL !== "/exception") {
+        this.lastTransition.abort();
+        this.setProperties({ lastTransition: null, thrown: null });
+        // Can't use routeTo because it handles navigation to the same page
+        DiscourseURL.handleURL(currentURL);
+      } else {
+        window.history.back();
+      }
     },
 
     tryLoading() {
       this.set("loading", true);
 
       schedule("afterRender", () => {
-        this.lastTransition.retry();
+        const transition = this.lastTransition;
+        this.setProperties({ lastTransition: null, thrown: null });
+        transition.retry();
         this.set("loading", false);
       });
     },

app/assets/javascripts/discourse/app/lib/url.js

@@ -483,6 +483,7 @@ const DiscourseURL = EmberObject.extend({
 
     transition._discourse_intercepted = true;
     transition._discourse_anchor = elementId;
+    transition._discourse_original_url = path;
 
     const promise = transition.promise || transition;
     promise.then(() => jumpToElement(elementId));

app/assets/javascripts/discourse/app/routes/application.js

@@ -94,13 +94,7 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
     },
 
     error(err, transition) {
-      let xhr = {};
-      if (err.jqXHR) {
-        xhr = err.jqXHR;
-      }
-
-      const xhrOrErr = err.jqXHR ? xhr : err;
-
+      const xhrOrErr = err.jqXHR ? err.jqXHR : err;
       const exceptionController = this.controllerFor("exception");
 
       const c = window.console;

app/assets/javascripts/discourse/app/templates/exception.hbs

@@ -5,9 +5,11 @@
     <div class="error-page">
       <div class="face">:(</div>
       <div class="reason">{{reason}}</div>
-      <div class="url">
-        {{i18n "errors.prev_page"}} <a href={{requestUrl}} data-auto-route="true">{{requestUrl}}</a>
-      </div>
+      {{#if requestUrl}}
+        <div class="url">
+          {{i18n "errors.prev_page"}} <a href={{requestUrl}} data-auto-route="true">{{requestUrl}}</a>
+        </div>
+      {{/if}}
       <div class="desc">
         {{#if networkFixed}}
           {{d-icon "check-circle"}}

app/controllers/application_controller.rb

@@ -299,7 +299,7 @@ class ApplicationController < ActionController::Base
       with_resolved_locale(check_current_user: false) do
         # Include error in HTML format for topics#show.
         if (request.params[:controller] == 'topics' && request.params[:action] == 'show') || (request.params[:controller] == 'categories' && request.params[:action] == 'find_by_slug')
-          opts[:extras] = { html: build_not_found_page(error_page_opts) }
+          opts[:extras] = { html: build_not_found_page(error_page_opts), group: error_page_opts[:group] }
         end
       end