Merge pull request #58 from Derpthemeus/refactor-error-handling

Overhaul error handling

routes/weather.ts

@@ -5,8 +5,9 @@ import * as SunCalc from "suncalc";
 import * as moment from "moment-timezone";
 import * as geoTZ from "geo-tz";
 
-import { AdjustmentOptions, GeoCoordinates, TimeData, WateringData, WeatherData, WeatherProvider } from "../types";
+import { AdjustmentOptions, GeoCoordinates, TimeData, WateringData, WeatherData } from "../types";
-const weatherProvider: WeatherProvider = require("./weatherProviders/" + ( process.env.WEATHER_PROVIDER || "OWM" ) ).default;
+import { WeatherProvider } from "./weatherProviders/WeatherProvider";
+const weatherProvider: WeatherProvider = new ( require("./weatherProviders/" + ( process.env.WEATHER_PROVIDER || "OWM" ) ).default )();
 
 // Define regex filters to match against location
 const filters = {
@@ -71,7 +72,8 @@ async function resolveCoordinates( location: string ): Promise< GeoCoordinates >
  * Makes an HTTP/HTTPS GET request to the specified URL and parses the JSON response body.
  * @param url The URL to fetch.
  * @return A Promise that will be resolved the with parsed response body if the request succeeds, or will be rejected
- * with an Error if the request or JSON parsing fails.
+ * with an error if the request or JSON parsing fails. This error may contain information about the HTTP request or,
+ * response including API keys and other sensitive information.
  */
 export async function httpJSONRequest(url: string ): Promise< any > {
 	try {
@@ -110,6 +112,7 @@ function getTimeData( coordinates: GeoCoordinates ): TimeData {
  * @param adjustmentOptions Options to tweak the calculation, or undefined/null if no custom values are to be used.
  * @param wateringData The weather to use to calculate watering percentage.
  * @return The percentage that watering should be scaled by.
+ * @throws An error message will be thrown if the watering scale cannot be calculated.
  */
 function calculateZimmermanWateringScale( adjustmentOptions: AdjustmentOptions, wateringData: WateringData ): number {
 
@@ -117,7 +120,7 @@ function calculateZimmermanWateringScale( adjustmentOptions: AdjustmentOptions,
 
 	// Check to make sure valid data exists for all factors
 	if ( !validateValues( [ "temp", "humidity", "precip" ], wateringData ) ) {
-		return 100;
+		throw "Necessary field(s) were missing from WateringData.";
 	}
 
 	// Get baseline conditions for 100% water level, if provided
@@ -181,11 +184,6 @@ function checkWeatherRestriction( adjustmentValue: number, weather: WateringData
 export const getWeatherData = async function( req: express.Request, res: express.Response ) {
 	const location: string = getParameter(req.query.loc);
 
-	if ( !weatherProvider.getWeatherData ) {
-		res.send( "Error: selected WeatherProvider does not support getWeatherData" );
-		return;
-	}
-
 	let coordinates: GeoCoordinates;
 	try {
 		coordinates = await resolveCoordinates( location );
@@ -196,7 +194,13 @@ export const getWeatherData = async function( req: express.Request, res: express
 
 	// Continue with the weather request
 	const timeData: TimeData = getTimeData( coordinates );
-	const weatherData: WeatherData = await weatherProvider.getWeatherData( coordinates );
+	let weatherData: WeatherData;
+	try {
+		weatherData = await weatherProvider.getWeatherData( coordinates );
+	} catch ( err ) {
+		res.send( "Error: " + err );
+		return;
+	}
 
 	res.json( {
 		...timeData,
@@ -221,6 +225,12 @@ export const getWateringData = async function( req: express.Request, res: expres
 		remoteAddress: string				= getParameter(req.headers[ "x-forwarded-for" ]) || req.connection.remoteAddress,
 		adjustmentOptions: AdjustmentOptions;
 
+	/* A message to include in the response to indicate that the watering scale was not calculated correctly and
+	defaulted to 100%. This approach is used for backwards compatibility because older OS firmware versions were
+	hardcoded to keep the previous watering scale if the response did not include a watering scale, but newer versions
+	might allow for different behaviors. */
+	let errorMessage: string = undefined;
+
 
 	// X-Forwarded-For header may contain more than one IP address and therefore
 	// the string is split against a comma and the first value is selected
@@ -254,18 +264,24 @@ export const getWateringData = async function( req: express.Request, res: expres
 	let timeData: TimeData = getTimeData( coordinates );
 	let wateringData: WateringData;
 	if ( adjustmentMethod !== ADJUSTMENT_METHOD.MANUAL || checkRestrictions ) {
-		if ( !weatherProvider.getWateringData ) {
+		try {
-			res.send( "Error: selected WeatherProvider does not support getWateringData" );
+			wateringData = await weatherProvider.getWateringData( coordinates );
+		} catch ( err ) {
+			res.send( "Error: " + err );
 			return;
 		}
-
-		wateringData = await weatherProvider.getWateringData( coordinates );
 	}
 
 	let scale = -1,	rainDelay = -1;
 
 	if ( adjustmentMethod === ADJUSTMENT_METHOD.ZIMMERMAN ) {
+		try {
 			scale = calculateZimmermanWateringScale( adjustmentOptions, wateringData );
+		} catch ( err ) {
+			// Default to a scale of 100% if the scale can't be calculated.
+			scale = 100;
+			errorMessage = err;
+		}
 	}
 
 	if (wateringData) {
@@ -296,7 +312,8 @@ export const getWateringData = async function( req: express.Request, res: expres
 		sunrise:	timeData.sunrise,
 		sunset:		timeData.sunset,
 		eip:		ipToInt( remoteAddress ),
-		rawData:	undefined
+		rawData:	undefined,
+		error:		errorMessage
 	};
 
 	if ( adjustmentMethod > ADJUSTMENT_METHOD.MANUAL ) {
@@ -318,7 +335,8 @@ export const getWateringData = async function( req: express.Request, res: expres
 			"&sunrise="		+	data.sunrise +
 			"&sunset="		+	data.sunset +
 			"&eip="			+	data.eip +
-			( data.rawData ? "&rawData=" + JSON.stringify( data.rawData ) : "" )
+			( data.rawData ? "&rawData=" + JSON.stringify( data.rawData ) : "" ) +
+			( errorMessage ? "&error=" + encodeURIComponent( errorMessage ) : "" )
 		);
 	}
 
@@ -328,7 +346,8 @@ export const getWateringData = async function( req: express.Request, res: expres
  * Makes an HTTP/HTTPS GET request to the specified URL and returns the response body.
  * @param url The URL to fetch.
  * @return A Promise that will be resolved the with response body if the request succeeds, or will be rejected with an
- * Error if the request fails.
+ * error if the request fails or returns a non-200 status code. This error may contain information about the HTTP
+ * request or, response including API keys and other sensitive information.
  */
 async function httpRequest( url: string ): Promise< string > {
 	return new Promise< any >( ( resolve, reject ) => {
@@ -343,6 +362,11 @@ async function httpRequest( url: string ): Promise< string > {
 		};
 
 		( isHttps ? https : http ).get( options, ( response ) => {
+			if ( response.statusCode !== 200 ) {
+				reject( `Received ${ response.statusCode } status code for URL '${ url }'.` );
+				return;
+			}
+
 			let data = "";
 
 			// Reassemble the data as it comes in

routes/weatherProviders/DarkSky.ts

@@ -1,9 +1,12 @@
 import * as moment from "moment-timezone";
 
-import { GeoCoordinates, WateringData, WeatherData, WeatherProvider } from "../../types";
+import { GeoCoordinates, WateringData, WeatherData } from "../../types";
 import { httpJSONRequest } from "../weather";
+import { WeatherProvider } from "./WeatherProvider";
 
-async function getDarkSkyWateringData( coordinates: GeoCoordinates ): Promise< WateringData > {
+export default class DarkSkyWeatherProvider extends WeatherProvider {
+
+	public async getWateringData( coordinates: GeoCoordinates ): Promise< WateringData > {
 		// The Unix timestamp of 24 hours ago.
 		const yesterdayTimestamp: number = moment().subtract( 1, "day" ).unix();
 		const todayTimestamp: number = moment().unix();
@@ -17,12 +20,12 @@ async function getDarkSkyWateringData( coordinates: GeoCoordinates ): Promise< W
 			yesterdayData = await httpJSONRequest( yesterdayUrl );
 			todayData = await httpJSONRequest( todayUrl );
 		} catch ( err ) {
-		// Indicate watering data could not be retrieved if an API error occurs.
+			console.error( "Error retrieving weather information from Dark Sky:", err );
-		return undefined;
+			throw "An error occurred while retrieving weather information from Dark Sky."
 		}
 
 		if ( !todayData.hourly || !todayData.hourly.data || !yesterdayData.hourly || !yesterdayData.hourly.data ) {
-		return undefined;
+			throw "Necessary field(s) were missing from weather information returned by Dark Sky.";
 		}
 
 		/* The number of hourly forecasts to use from today's data. This will only include elements that contain historic
@@ -39,7 +42,7 @@ async function getDarkSkyWateringData( coordinates: GeoCoordinates ): Promise< W
 
 		// Fail if not enough data is available.
 		if ( samples.length !== 24 ) {
-		return undefined;
+			throw "Insufficient data was returned by Dark Sky.";
 		}
 
 		const totals = { temp: 0, humidity: 0, precip: 0 };
@@ -58,7 +61,7 @@ async function getDarkSkyWateringData( coordinates: GeoCoordinates ): Promise< W
 		};
 	}
 
-async function getDarkSkyWeatherData( coordinates: GeoCoordinates ): Promise< WeatherData > {
+	public async getWeatherData( coordinates: GeoCoordinates ): Promise< WeatherData > {
 		const DARKSKY_API_KEY = process.env.DARKSKY_API_KEY,
 			forecastUrl = `https://api.darksky.net/forecast/${ DARKSKY_API_KEY }/${ coordinates[ 0 ] },${ coordinates[ 1 ] }?exclude=minutely,alerts,flags`;
 
@@ -66,12 +69,12 @@ async function getDarkSkyWeatherData( coordinates: GeoCoordinates ): Promise< We
 		try {
 			forecast = await httpJSONRequest( forecastUrl );
 		} catch ( err ) {
-		// Indicate weather data could not be retrieved if an API error occurs.
+			console.error( "Error retrieving weather information from Dark Sky:", err );
-		return undefined;
+			throw "An error occurred while retrieving weather information from Dark Sky."
 		}
 
 		if ( !forecast.currently || !forecast.daily || !forecast.daily.data ) {
-		return undefined;
+			throw "Necessary field(s) were missing from weather information returned by Dark Sky.";
 		}
 
 		const weather: WeatherData = {
@@ -104,10 +107,4 @@ async function getDarkSkyWeatherData( coordinates: GeoCoordinates ): Promise< We
 
 		return weather;
 	}
-
+}
-
-const DarkSkyWeatherProvider: WeatherProvider = {
-	getWateringData: getDarkSkyWateringData,
-	getWeatherData: getDarkSkyWeatherData
-};
-export default DarkSkyWeatherProvider;

routes/weatherProviders/OWM.ts

@@ -1,7 +1,10 @@
-import { GeoCoordinates, WateringData, WeatherData, WeatherProvider } from "../../types";
+import { GeoCoordinates, WateringData, WeatherData } from "../../types";
 import { httpJSONRequest } from "../weather";
+import { WeatherProvider } from "./WeatherProvider";
 
-async function getOWMWateringData( coordinates: GeoCoordinates ): Promise< WateringData > {
+export default class OWMWeatherProvider extends WeatherProvider {
+
+	public async getWateringData( coordinates: GeoCoordinates ): Promise< WateringData > {
 		const OWM_API_KEY = process.env.OWM_API_KEY,
 			forecastUrl = "http://api.openweathermap.org/data/2.5/forecast?appid=" + OWM_API_KEY + "&units=imperial&lat=" + coordinates[ 0 ] + "&lon=" + coordinates[ 1 ];
 
@@ -10,13 +13,13 @@ async function getOWMWateringData( coordinates: GeoCoordinates ): Promise< Water
 		try {
 			forecast = await httpJSONRequest( forecastUrl );
 		} catch ( err ) {
-		// Indicate watering data could not be retrieved if an API error occurs.
+			console.error( "Error retrieving weather information from OWM:", err );
-		return undefined;
+			throw "An error occurred while retrieving weather information from OWM."
 		}
 
 		// Indicate watering data could not be retrieved if the forecast data is incomplete.
 		if ( !forecast || !forecast.list ) {
-		return undefined;
+			throw "Necessary field(s) were missing from weather information returned by OWM.";
 		}
 
 		let totalTemp = 0,
@@ -39,7 +42,7 @@ async function getOWMWateringData( coordinates: GeoCoordinates ): Promise< Water
 		};
 	}
 
-async function getOWMWeatherData( coordinates: GeoCoordinates ): Promise< WeatherData > {
+	public async getWeatherData( coordinates: GeoCoordinates ): Promise< WeatherData > {
 		const OWM_API_KEY = process.env.OWM_API_KEY,
 			currentUrl = "http://api.openweathermap.org/data/2.5/weather?appid=" + OWM_API_KEY + "&units=imperial&lat=" + coordinates[ 0 ] + "&lon=" + coordinates[ 1 ],
 			forecastDailyUrl = "http://api.openweathermap.org/data/2.5/forecast/daily?appid=" + OWM_API_KEY + "&units=imperial&lat=" + coordinates[ 0 ] + "&lon=" + coordinates[ 1 ];
@@ -49,13 +52,13 @@ async function getOWMWeatherData( coordinates: GeoCoordinates ): Promise< Weathe
 			current = await httpJSONRequest( currentUrl );
 			forecast = await httpJSONRequest( forecastDailyUrl );
 		} catch ( err ) {
-		// Indicate watering data could not be retrieved if an API error occurs.
+			console.error( "Error retrieving weather information from OWM:", err );
-		return undefined;
+			throw "An error occurred while retrieving weather information from OWM."
 		}
 
 		// Indicate watering data could not be retrieved if the forecast data is incomplete.
 		if ( !current || !current.main || !current.wind || !current.weather || !forecast || !forecast.list ) {
-		return undefined;
+			throw "Necessary field(s) were missing from weather information returned by OWM.";
 		}
 
 		const weather: WeatherData = {
@@ -86,9 +89,4 @@ async function getOWMWeatherData( coordinates: GeoCoordinates ): Promise< Weathe
 
 		return weather;
 	}
-
+}
-const OWMWeatherProvider: WeatherProvider = {
-	getWateringData: getOWMWateringData,
-	getWeatherData: getOWMWeatherData
-};
-export default OWMWeatherProvider;

routes/weatherProviders/WeatherProvider.ts

@@ -0,0 +1,25 @@
+import { GeoCoordinates, WateringData, WeatherData } from "../../types";
+
+export class WeatherProvider {
+	/**
+	 * Retrieves weather data necessary for watering level calculations.
+	 * @param coordinates The coordinates to retrieve the watering data for.
+	 * @return A Promise that will be resolved with the WateringData if it is successfully retrieved,
+	 * or rejected with an error message if an error occurs while retrieving the WateringData or the WeatherProvider
+	 * does not support this method.
+	 */
+	getWateringData( coordinates : GeoCoordinates ): Promise< WateringData > {
+		throw "Selected WeatherProvider does not support getWateringData";
+	}
+
+	/**
+	 * Retrieves the current weather data for usage in the mobile app.
+	 * @param coordinates The coordinates to retrieve the weather for
+	 * @return A Promise that will be resolved with the WeatherData if it is successfully retrieved,
+	 * or rejected with an error message if an error occurs while retrieving the WeatherData or the WeatherProvider does
+	 * not support this method.
+	 */
+	getWeatherData( coordinates : GeoCoordinates ): Promise< WeatherData > {
+		throw "Selected WeatherProvider does not support getWeatherData";
+	}
+}

routes/weatherProviders/local.ts

@@ -1,6 +1,7 @@
 import * as express	from "express";
 import { CronJob } from "cron";
-import { GeoCoordinates, WateringData, WeatherProvider } from "../../types";
+import { GeoCoordinates, WateringData } from "../../types";
+import { WeatherProvider } from "./WeatherProvider";
 
 const count = { temp: 0, humidity: 0 };
 
@@ -43,7 +44,9 @@ export const captureWUStream = function( req: express.Request, res: express.Resp
 	res.send( "success\n" );
 };
 
-export const getLocalWateringData = function(): WateringData {
+export default class LocalWeatherProvider extends WeatherProvider {
+
+	public async getWateringData( coordinates: GeoCoordinates ): Promise< WateringData > {
 		const result: WateringData = {
 			...yesterday as WateringData,
 			// Use today's weather if we dont have information for yesterday yet (i.e. on startup)
@@ -59,6 +62,7 @@ export const getLocalWateringData = function(): WateringData {
 
 		return result;
 	};
+}
 
 new CronJob( "0 0 0 * * *", function() {
 
@@ -74,10 +78,3 @@ interface PWSStatus {
 	humidity?: number;
 	precip?: number;
 }
-
-const LocalWeatherProvider: WeatherProvider = {
-	getWateringData: async function ( coordinates: GeoCoordinates ) {
-		return getLocalWateringData();
-	}
-};
-export default LocalWeatherProvider;

types.ts

@@ -85,22 +85,4 @@ export interface AdjustmentOptions {
     d?: number;
 }
 
-export interface WeatherProvider {
-    /**
-     * Retrieves weather data necessary for watering level calculations.
-     * @param coordinates The coordinates to retrieve the watering data for.
-     * @return A Promise that will be resolved with the WateringData if it is successfully retrieved,
-     * or resolved with undefined if an error occurs while retrieving the WateringData.
-     */
-    getWateringData?( coordinates : GeoCoordinates ): Promise< WateringData >;
-
-    /**
-     * Retrieves the current weather data for usage in the mobile app.
-     * @param coordinates The coordinates to retrieve the weather for
-     * @return A Promise that will be resolved with the WeatherData if it is successfully retrieved,
-     * or resolved with undefined if an error occurs while retrieving the WeatherData.
-     */
-    getWeatherData?( coordinates : GeoCoordinates ): Promise< WeatherData >;
-}
-
 export type WeatherProviderId = "OWM" | "DarkSky" | "local";