Skip to content

Commit

Permalink
Listen to error event and throw errors from getData
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
alexmuller and apaleslimghost committed Jun 28, 2022
1 parent 7ace8dc commit 77814fb
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 1 deletion.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 11 additions & 0 deletions src/poller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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});
}
Expand All @@ -103,6 +110,10 @@ module.exports = EventEmitter => {
}

getData () {
if (this.error) {
throw this.error;
}

return this.data;
}
};
Expand Down
53 changes: 53 additions & 0 deletions tests/poller.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down

0 comments on commit 77814fb

Please sign in to comment.