From 77814fb7dc2c4bed3e066ffe6d5c5e6594772376 Mon Sep 17 00:00:00 2001
From: Alex Muller <alex.muller@ft.com>
Date: Tue, 28 Jun 2022 14:20:04 +0100
Subject: [PATCH] Listen to error event and throw errors from getData

In Node.js,  if nothing is listening to the 'error' event then
it will throw.

Because we were emitting this event inside a catch, this previously
would have rejected the promise. In Node 14 this logs an unhandled
promise rejection warning, but in Node 16 this behaviour will cause
the Node process to exit.

Instead, we are going to use the error event to store an error property,
and surface that to clients in the getData() method. This is a breaking
change and consumers will have to anticipate that getData() may now
throw.

Co-authored-by: Kara Brightwell <kara@153.io>
---
 README.md            |  2 +-
 src/poller.js        | 11 +++++++++
 tests/poller.spec.js | 53 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 87c3833..c61579f 100644
--- a/README.md
+++ b/README.md
@@ -38,7 +38,7 @@ parseData: function (data) {
 
 * `stop()` - Stops polling
 
-* `getData()` - Returns the last set of data retrieved from the server (post-processed if `parseData` function exists)
+* `getData()` - Returns the last set of data retrieved from the server (post-processed if `parseData` function exists). This will throw an `HttpError` if the most recent fetch received an error.
 
 #### Events
 
diff --git a/src/poller.js b/src/poller.js
index 0ab6ae4..3fdf52b 100644
--- a/src/poller.js
+++ b/src/poller.js
@@ -35,6 +35,12 @@ module.exports = EventEmitter => {
 			if (config.autostart) {
 				this.start ({initialRequest: true});
 			}
+
+			// We must listen to the error event to prevent throwing when we receive an HTTP error
+			// https://nodejs.org/docs/latest-v16.x/api/events.html#error-events
+			this.on('error', (error) => {
+				this.error = error;
+			});
 		}
 
 		isRunning () {
@@ -84,6 +90,7 @@ module.exports = EventEmitter => {
 					const latency = new Date () - time;
 					if (response.ok) {
 						this.emit ('ok', response, latency);
+						this.error = undefined;
 					} else {
 						throw new errors.HttpError({url:this.url, method:this.options.method || 'GET', response});
 					}
@@ -103,6 +110,10 @@ module.exports = EventEmitter => {
 		}
 
 		getData () {
+			if (this.error) {
+				throw this.error;
+			}
+
 			return this.data;
 		}
 	};
diff --git a/tests/poller.spec.js b/tests/poller.spec.js
index 81ef64b..39eb902 100644
--- a/tests/poller.spec.js
+++ b/tests/poller.spec.js
@@ -168,6 +168,59 @@ describe ('Poller', function () {
 
 	});
 
+	it ('Should throw from getData when fetch has received an HTTP error', function (done) {
+
+		const ft = nock ('http://example.com')
+			.get ('/')
+			.reply (503, {});
+
+		const p = new Poller({
+			url: 'http://example.com'
+		});
+
+		p.fetch ();
+
+		setTimeout (function () {
+			expect (ft.isDone ()).to.be.true; // ensure Nock has been used
+			expect (function () {
+				p.getData ();
+			}).to.throw ('HTTP Error 503 Service Unavailable');
+			done ();
+		}, 10);
+
+	});
+
+	it ('Should return data from getData when fetch has received an HTTP error followed by a success', function (done) {
+
+		const ft = nock ('http://example.com');
+
+		const p = new Poller({
+			url: 'http://example.com'
+		});
+
+		ft.get ('/').reply (503, '<h1>error</h1>');
+
+		p.fetch ();
+
+		setTimeout (function () {
+			expect (ft.isDone ()).to.be.true; // ensure Nock has been used
+			expect (function () {
+				p.getData ();
+			}).to.throw ('HTTP Error 503 Service Unavailable');
+
+			ft.get ('/').reply (200, '<h1>website</h1>')
+
+			p.fetch()
+
+			setTimeout (function () {
+				expect (p.getData ()).to.equal ('<h1>website</h1>');
+				done ();
+			}, 10);
+
+		}, 10);
+
+	});
+
 	it ('Should annotate the polling response with latency information', function (done) {
 
 		const ft = nock ('http://example.com')