Unreliable, potential behavior changes in Node or other dependencies

[jesec]

Environment:

Ubuntu 20.04.1 LTS
Node v15.0.1
feedsub 0.7.2

I found the feedsub becomes unreliable recently.

Specifically, success() is never called:

node-feedsub/lib/feedsub.js

Line 122 in 34057ed

const success = (abort) => {

I found the issue is related to:

node-feedsub/lib/feedsub.js

Line 293 in 34057ed

req.on('end', () => {

AFAIK, it should be either req.on('close') or res.on('end'). req.on('end') is probably not the right one.

This patch seems to fix the problem:

-      req.on('end', () => {
+      req.on('close', () => {

I guess there are behavior changes in Node or other dependencies.

[fent]

end is emitted from miniget, which this library uses.

I just ran this tiny script on node versions 10 to 15, and the items do print

const FeedSub = require('feedsub');

let reader = new FeedSub('http://rss.cnn.com/rss/cnn_latest.rss', {
  interval: 1,
  autoStart: true,
  emitOnStart: true,
});

reader.on('item', (item) => {
  console.log('Got item!');
  console.dir(item);
});

reader.start();

[jesec]

I patiently waited for 5 minutes and there is no luck.

[jesec]

I do think there are some unknown behavior changes in the dependency graph. It works right away with req.on('close'.

[fent]

does it work with res.on('end'? that's res for response

can you post your npm ls output to see exactly what versions of feedsub and its dependencies it's installing?

[jesec]

No, res.on('end' does not work.

[jesec]

FYI: Some of the breaking behavior changes of Node 15:
[2868f52a5c] - stream: fix regression on duplex end (Momtchil Momtchev) #35941
[70c41a830d] - stream: remove redundant context from comments (Yash Ladha) #35728
[88eb6191e4] - stream: fix duplicate logic in stream destroy (Yash Ladha) #35727
[a65218f5e8] - (SEMVER-MAJOR) stream: try to wait for flush to complete before 'finish' (Robert Nagy) #34314
[4e3f6f355b] - (SEMVER-MAJOR) stream: cleanup and fix Readable.wrap (Robert Nagy) #34204
[527e2147af] - (SEMVER-MAJOR) stream: add promises version to utility functions (rickyes) #33991
[c7e55c6b72] - (SEMVER-MAJOR) stream: fix writable.end callback behavior (Robert Nagy) #34101
[fb8cc72e73] - (SEMVER-MAJOR) stream: construct (Robert Nagy) #29656
[4bc7025309] - (SEMVER-MAJOR) stream: write should throw on unknown encoding (Robert Nagy) #33075
[ea87809bb6] - (SEMVER-MAJOR) stream: fix _final and 'prefinish' timing (Robert Nagy) #32780
[0bd5595509] - (SEMVER-MAJOR) stream: simplify Transform stream implementation (Robert Nagy) #32763
[8f86986985] - (SEMVER-MAJOR) stream: use callback to properly propagate error (Robert Nagy) #29179
[744a284ccc] - (SEMVER-MINOR) stream: support async for stream impl functions (James M Snell) #34416
[6be80e1893] - stream: fix legacy pipe error handling (Robert Nagy) #35257
[2b9003b165] - stream: don't destroy on async iterator success (Robert Nagy) #35122
[9c62e0e384] - stream: move to internal/streams (Matteo Collina) #35239
[e0d3b758a0] - stream: improve Writable.destroy performance (Robert Nagy) #35067
[02c4869bee] - stream: fix Duplex._construct race (Robert Nagy) #34456
[5aeaff6499] - stream: refactor lazyLoadPromises (rickyes) #34354
[a55b77d2d3] - stream: finished on closed OutgoingMessage (Robert Nagy) #34313
[e10e292c5e] - stream: remove unused _transformState (Robert Nagy) #33105
[f5c11a1a0a] - stream: don't emit finish after close (Robert Nagy) #32933

I marked the suspicious ones in bold.

ref: https://github.com/nodejs/node/releases

I think req.on('close' is probably the way to go after all.