OK, now you have more of the whole code so I can see what you're trying to accomplish.
A few issues I see:
- The variable
a
is not defined, so it's an implicit global. That's bad. Declare it somewhere local.
- When computing the combined content-length, you have a complete race condition between your two requests. Your code is assuming that
adsys
gives you a response
first, but that is not guaranteed to happen.
- You compute that length, but don't actually use it. You can't put it in your
res.set()
instruction because you haven't necessarily finished calculating it yet when that runs.
- It seems like you're missing error handling on at least the adsys request.
- It appears to me in the needle documentation that the completion event for a needle request is
"done"
, not "finish"
.
Here's a significantly cleaned up version of your code with a bunch of new logging to help debug:
let trackCntr = 0;
function getLogger() {
let id = ++trackCntr;
return function(...args) {
args.unshift(`(${id}): `);
console.log(...args);
}
}
router.get('/track/:url(*)', (req, res) => {
const log = getLogger();
const url = req.params.url.substr(0);
log(`(${trackCntr}) /track/${url}`);
/* AD SYS */
const remote = "https://storage.googleapis.com/ad-system/testfolder/OUTOFAREA.mp3";
const adsys = needle.get(remote);
/* PODCAST */
const filesize = needle.get(url);
let responseCntr = 0;
let length = 0;
let errReportedAlready = false;
adsys.on("response", function(resB) {
log(`adsys.on('response'), content-length=${resB.headers['content-length']}`);
length += +resB.headers['content-length'];
++responseCntr;
checkResponseCntr();
});
adsys.on("err", sendErr);
adsys.on("timeout", sendErr);
filesize.on("response", function(resC) {
log(`filesize.on('response'), content-length=${resC.headers['content-length']}`);
length += +resC.headers['content-length'];
++responseCntr;
checkResponseCntr();
});
filesize.on("err", sendErr);
filesize.on("timeout", sendErr);
// this is called if either needle requests gets an error
function sendErr(err) {
log("sendErr", err);
if (!errReportedAlready) {
errReportedAlready = true;
if (res.headersSent) {
// just need to abort the response because headers have already been sent
res.end();
} else {
// send error status
res.sendStatus(500);
}
}
}
// code continues here after both response headers above have completed
function checkResponseCntr() {
log(`checkResponseCntr(${responseCntr})`)
// if we have received both responses, then start streaming ad data
if (responseCntr === 2) {
log("got both responses");
res.set({
"Content-Type": "audio/mpeg",
"Transfer-Encoding": "chunk",
"Content-Length": length
});
// start streaming ad data
getAd();
}
}
function getAd() {
log("getAd()");
// this will cause adsys data to start flowing
adsys.on("data", function(chunk) {
if (!errReportedAlready) {
res.write(chunk);
}
});
adsys.on("done", function() {
log("adsys done");
// now trigger getting the podcast data
getPodcast();
});
}
function getPodcast() {
log("getPodcast()");
filesize.on("data", function(chunk) {
if (!errReportedAlready) {
res.write(chunk);
}
});
filesize.on("done", function() {
log("filesize done");
if (!errReportedAlready) {
log("res.end()")
res.end();
}
});
}
});
module.exports = router;
This does the following:
- Correctly compute the length without regard to race condition order of the two requests.
- Add error handling for both
needle()
requests.
- Add proper setting of
content-length
header.
- Change monitoring of
needle()
complete to use the "done"
event per the needle()
documentation.
- Code
getAd()
and getPodcast()
similarly.
Possible issues still:
- If it takes a long time to stream the ad, I could imagine your filesize request timing out.
I'm able to run the above code in my own little nodejs app and this is the logging I get:
(1): (1) /track/https://storage.googleapis.com/radiomediapodcast/wellwellnow/season1/S01E04.mp3
(1): adsys.on('response'), content-length=754542
(1): checkResponseCntr(1)
(1): filesize.on('response'), content-length=63062853
(1): checkResponseCntr(2)
(1): got both responses
(1): getAd()
(1): adsys done
(1): getPodcast()
(2): (2) /track/https://storage.googleapis.com/radiomediapodcast/wellwellnow/season1/S01E04.mp3
(2): adsys.on('response'), content-length=754542
(2): checkResponseCntr(1)
(2): filesize.on('response'), content-length=63062853
(2): checkResponseCntr(2)
(2): got both responses
(2): getAd()
(2): adsys done
(2): getPodcast()
(2): filesize done
(2): res.end()
(1): filesize done
(1): res.end()
You can clearly see the two separate requests coming in. The second request arrives as soon as the first request sends it's headers - not sure why.
I've determined that the double request is caused by putting the audio URL into the Chrome URL bar. If I put that URL into an audio tag in an HTML page, then we no longer get the double requests. I created this simple HTML page:
<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title></title>
</head>
<body>
<figure>
<figcaption>Listen to the T-Rex:</figcaption>
<audio
controls
src="http://localhost/track/https%3A%2F%2Fstorage.googleapis.com%2Fradiomediapodcast%2Fwellwellnow%2Fseason1%2FS01E04.mp3">
Your browser does not support the
<code>audio</code> element.
</audio>
</figure>
</body>
</html>
And, then I get just this log (which looks right to me):
(1): (1) /track/https://storage.googleapis.com/radiomediapodcast/wellwellnow/season1/S01E04.mp3
(1): Accept: */*
(1): adsys.on('response'), content-length=754542
(1): checkResponseCntr(1)
(1): filesize.on('response'), content-length=63062853
(1): checkResponseCntr(2)
(1): got both responses
(1): getAd()
(1): adsys done
(1): getPodcast()
(1): filesize done
(1): res.end()
And, the double request seems to be a Chrome/Edge URL bar thing because it does not happen in Firefox when I put the URL into the Firefox URL bar.