Merge pull request #1845 from thelounge/astorije/error-loading

Improve error page at loading time

client/css/style.css

@@ -160,6 +160,7 @@ kbd {
 }
 
 #js-copy-hack,
+#loading pre,
 #help,
 #windows .header .title,
 #windows .header .topic,
@@ -1354,10 +1355,24 @@ part/quit messages where we don't load previews (adds a blank line otherwise) */
 	margin-top: 10px;
 }
 
-#loading-slow {
+#loading-slow,
+#loading-reload {
 	display: none;
 }
 
+#loading-reload {
+	margin-top: 15px;
+}
+
+#loading summary {
+	outline: none;
+	cursor: pointer;
+}
+
+#loading pre {
+	white-space: normal;
+}
+
 #sign-in label {
 	display: block;
 	margin-top: 10px;

client/index.html

@@ -53,11 +53,12 @@
 							</div>
 							<div class="col-xs-12">
 								<p id="loading-page-message">Loading the app… <a href="http://enable-javascript.com/" target="_blank" rel="noopener">Make sure to have JavaScript enabled.</a></p>
-								<div id="loading-slow">
-									<p>This is taking longer than it should, there might be connectivity issues.</p>
-									<button id="loading-slow-reload" class="btn">Reload page</button>
-								</div>
-								<script async src="js/loading-slow-alert.js"></script>
+								<p id="loading-slow">
+									This is taking longer than it should, there might be
+									connectivity issues.
+								</p>
+								<button id="loading-reload" class="btn">Reload page</button>
+								<script async src="js/loading-error-handlers.js"></script>
 							</div>
 						</div>
 					</div>

client/js/loading-error-handlers.js

@@ -0,0 +1,63 @@
+/* eslint strict: 0 */
+"use strict";
+
+/*
+ * This is a separate file for two reasons:
+ * 1. CSP policy does not allow inline javascript
+ * 2. It has to be a small javascript executed before all other scripts,
+ *    so that the timeout can be triggered while slow JS is loading
+ */
+
+(function() {
+	var displayReload = function displayReload() {
+		var loadingReload = document.getElementById("loading-reload");
+		if (loadingReload) {
+			loadingReload.style.display = "block";
+		}
+	};
+
+	var loadingSlowTimeout = setTimeout(function() {
+		var loadingSlow = document.getElementById("loading-slow");
+
+		// The parent element, #loading, is being removed when the app is loaded.
+		// Since the timer is not cancelled, `loadingSlow` can be not found after
+		// 5s. Wrap everything in this block to make sure nothing happens if the
+		// element does not exist (i.e. page has loaded).
+		if (loadingSlow) {
+			loadingSlow.style.display = "block";
+			displayReload();
+		}
+	}, 5000);
+
+	document.getElementById("loading-reload").addEventListener("click", function() {
+		location.reload();
+	});
+
+	window.g_LoungeErrorHandler = function LoungeErrorHandler(e) {
+		var title = document.getElementById("loading-title");
+		title.textContent = "An error has occured";
+
+		var message = document.getElementById("loading-page-message");
+		message.textContent = "An error has occured that prevented the client from loading correctly.";
+
+		var summary = document.createElement("summary");
+		summary.textContent = "More details";
+
+		var data = document.createElement("pre");
+		data.textContent = e.message; // e is an ErrorEvent
+
+		var info = document.createElement("p");
+		info.textContent = "Open the developer tools of your browser for more information.";
+
+		var details = document.createElement("details");
+		details.appendChild(summary);
+		details.appendChild(data);
+		details.appendChild(info);
+		message.parentNode.insertBefore(details, message.nextSibling);
+
+		window.clearTimeout(loadingSlowTimeout);
+		displayReload();
+	};
+
+	window.addEventListener("error", window.g_LoungeErrorHandler);
+})();

client/js/loading-slow-alert.js

@@ -1,47 +0,0 @@
-/* eslint strict: 0 */
-"use strict";
-
-/*
- * This is a separate file for two reasons:
- * 1. CSP policy does not allow inline javascript
- * 2. It has to be a small javascript executed before all other scripts,
- *    so that the timeout can be triggered while slow JS is loading
- */
-
-setTimeout(function() {
-	var element = document.getElementById("loading-slow");
-
-	if (element) {
-		element.style.display = "block";
-	}
-}, 5000);
-
-document.getElementById("loading-slow-reload").addEventListener("click", function() {
-	location.reload();
-});
-
-window.g_LoungeErrorHandler = function LoungeErrorHandler(error) {
-	var title = document.getElementById("loading-title");
-	title.textContent = "An error has occured";
-
-	title = document.getElementById("loading-page-message");
-	title.textContent = "An error has occured that prevented the client from loading correctly.";
-
-	var summary = document.createElement("summary");
-	summary.textContent = "More details";
-
-	if (error instanceof ErrorEvent) {
-		error = error.message + "\n\n" + error.stack + "\n\nView developer tools console for more information and a better stacktrace.";
-	}
-
-	var data = document.createElement("pre");
-	data.contentEditable = true;
-	data.textContent = error;
-
-	var details = document.createElement("details");
-	details.appendChild(summary);
-	details.appendChild(data);
-	title.parentNode.insertBefore(details, title.nextSibling);
-};
-
-window.addEventListener("error", window.g_LoungeErrorHandler);

test/tests/build.js

@@ -25,7 +25,7 @@ describe("public folder", function() {
 		expect(fs.existsSync(path.join(publicFolder, "js", "bundle.js.map"))).to.be.true;
 	});
 
-	it("loading-slow-alert.js is copied", function() {
-		expect(fs.existsSync(path.join(publicFolder, "js", "loading-slow-alert.js"))).to.be.true;
+	it("loading-error-handlers.js is copied", function() {
+		expect(fs.existsSync(path.join(publicFolder, "js", "loading-error-handlers.js"))).to.be.true;
 	});
 });

webpack.config.js

@@ -67,7 +67,7 @@ const config = {
 				to: "fonts/[name].[ext]",
 			},
 			{
-				from: "./client/js/loading-slow-alert.js",
+				from: "./client/js/loading-error-handlers.js",
 				to: "js/[name].[ext]",
 			},
 			{ // TODO: Build index.html with handlebars