Merge pull request #2552 from rejas/error_handling

CHANGELOG.md

@@ -30,6 +30,7 @@ Special thanks to the following contributors: @B1gG, @codac, @ezeholz, @khassel,
 - Moved some files into better suited directories
 - Update dependencies in package.json, require node >= v12, remove `rrule-alt` and `rrule`
 - Update dependencies in package.json and migrate husky to v6
+- Cleaned up error handling in newsfeed and calendar modules
 
 ### Removed
 

js/node_helper.js

@@ -113,6 +113,32 @@ const NodeHelper = Class.extend({
 	}
 });
 
+NodeHelper.checkFetchStatus = function (response) {
+	// response.status >= 200 && response.status < 300
+	if (response.ok) {
+		return response;
+	} else {
+		throw Error(response.statusText);
+	}
+};
+
+/**
+ * Look at the specified error and return an appropriate error type, that
+ * can be translated to a detailed error message
+ *
+ * @param {Error} error the error from fetching something
+ * @returns {string} the string of the detailed error message in the translations
+ */
+NodeHelper.checkFetchError = function (error) {
+	let error_type = "MODULE_ERROR_UNSPECIFIED";
+	if (error.code === "EAI_AGAIN") {
+		error_type = "MODULE_ERROR_NO_CONNECTION";
+	} else if (error.message === "Unauthorized") {
+		error_type = "MODULE_ERROR_UNAUTHORIZED";
+	}
+	return error_type;
+};
+
 NodeHelper.create = function (moduleDefinition) {
 	return NodeHelper.extend(moduleDefinition);
 };

modules/default/alert/notificationFx.js

@@ -122,6 +122,8 @@
 
 	/**
 	 * Dismiss the notification
+	 *
+	 * @param {boolean} [close] call the onClose callback at the end
 	 */
 	NotificationFx.prototype.dismiss = function (close = true) {
 		this.active = false;

modules/default/calendar/calendar.js

@@ -146,11 +146,10 @@ Module.register("calendar", {
 					this.broadcastEvents();
 				}
 			}
-		} else if (notification === "FETCH_ERROR") {
+		} else if (notification === "CALENDAR_ERROR") {
-			Log.error("Calendar Error. Could not fetch calendar: " + payload.url);
+			let error_message = this.translate(payload.error_type);
+			this.error = this.translate("MODULE_CONFIG_ERROR", { MODULE_NAME: this.name, ERROR: error_message });
 			this.loaded = true;
-		} else if (notification === "INCORRECT_URL") {
-			Log.error("Calendar Error. Incorrect url: " + payload.url);
 		}
 
 		this.updateDom(this.config.animationSpeed);
@@ -168,6 +167,12 @@ Module.register("calendar", {
 		const wrapper = document.createElement("table");
 		wrapper.className = this.config.tableClass;
 
+		if (this.error) {
+			wrapper.innerHTML = this.error;
+			wrapper.className = this.config.tableClass + " dimmed";
+			return wrapper;
+		}
+
 		if (events.length === 0) {
 			wrapper.innerHTML = this.loaded ? this.translate("EMPTY") : this.translate("LOADING");
 			wrapper.className = this.config.tableClass + " dimmed";

modules/default/calendar/calendarfetcher.js

@@ -6,6 +6,7 @@
  */
 const CalendarUtils = require("./calendarutils");
 const Log = require("logger");
+const NodeHelper = require("node_helper");
 const ical = require("node-ical");
 const fetch = require("node-fetch");
 const digest = require("digest-fetch");
@@ -62,17 +63,7 @@ const CalendarFetcher = function (url, reloadInterval, excludedEvents, maximumEn
 		}
 
 		fetcher
-			.catch((error) => {
+			.then(NodeHelper.checkFetchStatus)
-				fetchFailedCallback(this, error);
-				scheduleTimer();
-			})
-			.then((response) => {
-				if (response.status !== 200) {
-					fetchFailedCallback(this, response.statusText);
-					scheduleTimer();
-				}
-				return response;
-			})
 			.then((response) => response.text())
 			.then((responseData) => {
 				let data = [];
@@ -87,12 +78,16 @@ const CalendarFetcher = function (url, reloadInterval, excludedEvents, maximumEn
 						maximumNumberOfDays
 					});
 				} catch (error) {
-					fetchFailedCallback(this, error.message);
+					fetchFailedCallback(this, error);
 					scheduleTimer();
 					return;
 				}
 				this.broadcastEvents();
 				scheduleTimer();
+			})
+			.catch((error) => {
+				fetchFailedCallback(this, error);
+				scheduleTimer();
 			});
 	};
 

modules/default/calendar/calendarutils.js

@@ -18,8 +18,8 @@ const CalendarUtils = {
 	 * Calculate the time correction, either dst/std or full day in cases where
 	 * utc time is day before plus offset
 	 *
-	 * @param {object} event
+	 * @param {object} event the event which needs adjustement
-	 * @param {Date} date
+	 * @param {Date} date the date on which this event happens
 	 * @returns {number} the necessary adjustment in hours
 	 */
 	calculateTimezoneAdjustment: function (event, date) {
@@ -117,6 +117,13 @@ const CalendarUtils = {
 		return adjustHours;
 	},
 
+	/**
+	 * Filter the events from ical according to the given config
+	 *
+	 * @param {object} data the calendar data from ical
+	 * @param {object} config The configuration object
+	 * @returns {string[]} the filtered events
+	 */
 	filterEvents: function (data, config) {
 		const newEvents = [];
 
@@ -500,8 +507,8 @@ const CalendarUtils = {
 	/**
 	 * Lookup iana tz from windows
 	 *
-	 * @param msTZName
+	 * @param {string} msTZName the timezone name to lookup
-	 * @returns {*|null}
+	 * @returns {string|null} the iana name or null of none is found
 	 */
 	getIanaTZFromMS: function (msTZName) {
 		// Get hash entry
@@ -571,12 +578,13 @@ const CalendarUtils = {
 	},
 
 	/**
+	 * Determines if the user defined title filter should apply
 	 *
-	 * @param title
+	 * @param {string} title the title of the event
-	 * @param filter
+	 * @param {string} filter the string to look for, can be a regex also
-	 * @param useRegex
+	 * @param {boolean} useRegex true if a regex should be used, otherwise it just looks for the filter as a string
-	 * @param regexFlags
+	 * @param {string} regexFlags flags that should be applied to the regex
-	 * @returns {boolean|*}
+	 * @returns {boolean} True if the title should be filtered out, false otherwise
 	 */
 	titleFilterApplies: function (title, filter, useRegex, regexFlags) {
 		if (useRegex) {

modules/default/calendar/node_helper.js

@@ -40,13 +40,14 @@ module.exports = NodeHelper.create({
 		try {
 			new URL(url);
 		} catch (error) {
-			this.sendSocketNotification("INCORRECT_URL", { id: identifier, url: url });
+			Log.error("Calendar Error. Malformed calendar url: ", url, error);
+			this.sendSocketNotification("CALENDAR_ERROR", { error_type: "MODULE_ERROR_MALFORMED_URL" });
 			return;
 		}
 
 		let fetcher;
 		if (typeof this.fetchers[identifier + url] === "undefined") {
-			Log.log("Create new calendar fetcher for url: " + url + " - Interval: " + fetchInterval);
+			Log.log("Create new calendarfetcher for url: " + url + " - Interval: " + fetchInterval);
 			fetcher = new CalendarFetcher(url, fetchInterval, excludedEvents, maximumEntries, maximumNumberOfDays, auth, broadcastPastEvents, selfSignedCert);
 
 			fetcher.onReceive((fetcher) => {
@@ -55,16 +56,16 @@ module.exports = NodeHelper.create({
 
 			fetcher.onError((fetcher, error) => {
 				Log.error("Calendar Error. Could not fetch calendar: ", fetcher.url(), error);
-				this.sendSocketNotification("FETCH_ERROR", {
+				let error_type = NodeHelper.checkFetchError(error);
+				this.sendSocketNotification("CALENDAR_ERROR", {
 					id: identifier,
-					url: fetcher.url(),
+					error_type
-					error: error
 				});
 			});
 
 			this.fetchers[identifier + url] = fetcher;
 		} else {
-			Log.log("Use existing calendar fetcher for url: " + url);
+			Log.log("Use existing calendarfetcher for url: " + url);
 			fetcher = this.fetchers[identifier + url];
 			fetcher.broadcastEvents();
 		}

modules/default/newsfeed/newsfeed.js

@@ -89,8 +89,8 @@ Module.register("newsfeed", {
 
 			this.loaded = true;
 			this.error = null;
-		} else if (notification === "INCORRECT_URL") {
+		} else if (notification === "NEWSFEED_ERROR") {
-			this.error = `Incorrect url: ${payload.url}`;
+			this.error = this.translate(payload.error_type);
 			this.scheduleUpdateInterval();
 		}
 	},
@@ -183,9 +183,9 @@ Module.register("newsfeed", {
 		}
 
 		if (this.config.prohibitedWords.length > 0) {
-			newsItems = newsItems.filter(function (value) {
+			newsItems = newsItems.filter(function (item) {
 				for (let word of this.config.prohibitedWords) {
-					if (value["title"].toLowerCase().indexOf(word.toLowerCase()) > -1) {
+					if (item.title.toLowerCase().indexOf(word.toLowerCase()) > -1) {
 						return false;
 					}
 				}

modules/default/newsfeed/newsfeedfetcher.js

@@ -6,6 +6,7 @@
  */
 const Log = require("logger");
 const FeedMe = require("feedme");
+const NodeHelper = require("node_helper");
 const fetch = require("node-fetch");
 const iconv = require("iconv-lite");
 
@@ -84,12 +85,13 @@ const NewsfeedFetcher = function (url, reloadInterval, encoding, logFeedWarnings
 		};
 
 		fetch(url, { headers: headers })
+			.then(NodeHelper.checkFetchStatus)
+			.then((response) => {
+				response.body.pipe(iconv.decodeStream(encoding)).pipe(parser);
+			})
 			.catch((error) => {
 				fetchFailedCallback(this, error);
 				scheduleTimer();
-			})
-			.then((res) => {
-				res.body.pipe(iconv.decodeStream(encoding)).pipe(parser);
 			});
 	};
 

modules/default/newsfeed/node_helper.js

@@ -27,8 +27,8 @@ module.exports = NodeHelper.create({
 	 * Creates a fetcher for a new feed if it doesn't exist yet.
 	 * Otherwise it reuses the existing one.
 	 *
-	 * @param {object} feed The feed object.
+	 * @param {object} feed The feed object
-	 * @param {object} config The configuration object.
+	 * @param {object} config The configuration object
 	 */
 	createFetcher: function (feed, config) {
 		const url = feed.url || "";
@@ -38,13 +38,14 @@ module.exports = NodeHelper.create({
 		try {
 			new URL(url);
 		} catch (error) {
-			this.sendSocketNotification("INCORRECT_URL", { url: url });
+			Log.error("Newsfeed Error. Malformed newsfeed url: ", url, error);
+			this.sendSocketNotification("NEWSFEED_ERROR", { error_type: "MODULE_ERROR_MALFORMED_URL" });
 			return;
 		}
 
 		let fetcher;
 		if (typeof this.fetchers[url] === "undefined") {
-			Log.log("Create new news fetcher for url: " + url + " - Interval: " + reloadInterval);
+			Log.log("Create new newsfetcher for url: " + url + " - Interval: " + reloadInterval);
 			fetcher = new NewsfeedFetcher(url, reloadInterval, encoding, config.logFeedWarnings);
 
 			fetcher.onReceive(() => {
@@ -52,15 +53,16 @@ module.exports = NodeHelper.create({
 			});
 
 			fetcher.onError((fetcher, error) => {
-				this.sendSocketNotification("FETCH_ERROR", {
+				Log.error("Newsfeed Error. Could not fetch newsfeed: ", url, error);
-					url: fetcher.url(),
+				let error_type = NodeHelper.checkFetchError(error);
-					error: error
+				this.sendSocketNotification("NEWSFEED_ERROR", {
+					error_type
 				});
 			});
 
 			this.fetchers[url] = fetcher;
 		} else {
-			Log.log("Use existing news fetcher for url: " + url);
+			Log.log("Use existing newsfetcher for url: " + url);
 			fetcher = this.fetchers[url];
 			fetcher.setReloadInterval(reloadInterval);
 			fetcher.broadcastItems();

tests/configs/data/feed_test_rodrigoramirez.xml

@@ -1,15 +1,15 @@
-<?xml version="1.0" encoding="UTF-8"?><rss version="2.0"
+<?xml version="1.0" encoding="UTF-8"?>
+<rss version="2.0"
      xmlns:content="http://purl.org/rss/1.0/modules/content/"
      xmlns:wfw="http://wellformedweb.org/CommentAPI/"
      xmlns:dc="http://purl.org/dc/elements/1.1/"
      xmlns:atom="http://www.w3.org/2005/Atom"
      xmlns:sy="http://purl.org/rss/1.0/modules/syndication/"
      xmlns:slash="http://purl.org/rss/1.0/modules/slash/"
-	>
+>
-
+  <channel>
-<channel>
     <title>Rodrigo Ramírez Norambuena</title>
-	<atom:link href="https://rodrigoramirez.com/feed/" rel="self" type="application/rss+xml" />
+    <atom:link href="https://rodrigoramirez.com/feed/" rel="self" type="application/rss+xml"/>
     <link>https://rodrigoramirez.com</link>
     <description>Temas sobre Linux, VoIP, Open Source, tecnología y lo relacionado.</description>
     <lastBuildDate>Fri, 21 Oct 2016 21:30:22 +0000</lastBuildDate>

tests/e2e/modules/calendar_spec.js

@@ -149,8 +149,8 @@ describe("Calendar module", function () {
 			serverBasicAuth.close(done());
 		});
 
-		it("should return No upcoming events", function () {
+		it("should show Unauthorized error", function () {
-			return app.client.waitUntilTextExists(".calendar", "No upcoming events.", 10000);
+			return app.client.waitUntilTextExists(".calendar", "Error in the calendar module. Authorization failed", 10000);
 		});
 	});
 });

tests/e2e/modules/newsfeed_spec.js

@@ -66,8 +66,8 @@ describe("Newsfeed module", function () {
 			process.env.MM_CONFIG_FILE = "tests/configs/modules/newsfeed/incorrect_url.js";
 		});
 
-		it("should show invalid url warning", function () {
+		it("should show malformed url warning", function () {
-			return app.client.waitUntilTextExists(".newsfeed .small", "Error in the Newsfeed module. Incorrect url:", 10000);
+			return app.client.waitUntilTextExists(".newsfeed .small", "Error in the Newsfeed module. Malformed url.", 10000);
 		});
 	});
 });

translations/en.json

@@ -30,6 +30,10 @@
 
 	"MODULE_CONFIG_CHANGED": "The configuration options for the {MODULE_NAME} module have changed.\nPlease check the documentation.",
 	"MODULE_CONFIG_ERROR": "Error in the {MODULE_NAME} module. {ERROR}",
+	"MODULE_ERROR_MALFORMED_URL": "Malformed url.",
+	"MODULE_ERROR_NO_CONNECTION": "No internet connection.",
+	"MODULE_ERROR_UNAUTHORIZED": "Authorization failed.",
+	"MODULE_ERROR_UNSPECIFIED": "Check logs for more details.",
 
 	"UPDATE_NOTIFICATION": "MagicMirror² update available.",
 	"UPDATE_NOTIFICATION_MODULE": "Update available for {MODULE_NAME} module.",