I'm not an expert warning

I like the change,
not sure about the issue.

maybe you are right, it'll fail eventually
not because of the redundant await
but because you dont let resolve/resume the 1st of the 2 calls on every call.

...
I never liked arbitrary recursivity, you dont let variables die
shouldnt we kill every "response" too?

or

let response;
subscribe();
// is dirty, I know

I'm not an expert either warning. ;)

It would probably take a long time to overflow the stack, depending on how often the poll succeeds or times out. but I think it's worth mentioning. Normally when you do recursion, you want to keep the result of each successive call. For example, to calculate a factorial:

function factorial(n) {
 if (n>1) {
 	return n*factorial(n-1);
 } else {
 	return 1;
 }
}
console.log( factorial(7) )

In this example I'm using the stack as an accumulator to calculate a factorial. This also creates a hard limit on how many times the recursion will occur.

However, with long polling there's no limit on how many times the recursion will happen. It will keep happening as long as the page is open and the server is responding. Also, we don't care about the return value of the next call. The subscribe() function always returns void so there's no reason to await for the promise to resolve.

To see what I mean, add a counter to keep track of each call:

async function subscribe(n) {
 console.log("Starting "+n);
 
 // Do stuff
 // ...
 await subscribe(n+1);
 
 console.log("Ending "+n);
}
subscribe(1);

Your console will look like this:

"Starting 1"
"Starting 2"
"Starting 3"
"Starting 4"
...

This will continue forever and you will never see the phrase "Ending". All of those calls will build up in the stack and never have a chance to "unwind". But if you remove the await keyword, your console will look like this:

"Starting 1"
"Starting 2"
"Ending 1"
"Starting 3"
"Ending 2"
"Starting 4"
"Ending 3"
"Starting 5"
"Ending 4"
...

It will still go on forever, but the previous call ends right after the next call starts. This frees up resources and prevents the stack from overflowing.

well, this is only browser code, it'll be hard to reach the hard limit

maybe a warning
"dont think it is a good idea to code like this in a forever runing node server"

about the await,
yes, I think the only one needed is called at the top ofthe function

I don't think there are any downsides to removing the await. But since neither of us is an expert, maybe someone else will offer their wisdom.

So after more research I found something called Tail Call Optimization. However, browser support for it is spotty at best so now I think the best solution is to not use recursion at all and just use a loop:

async function subscribe() {
 
 while (true) {
 let response = await fetch("/subscribe");
 if (response.status == 200) {
 // Get and show the message
 let message = await response.text();
 showMessage(message);
 } else if (response.status == 502) {
 // Status 502 is a connection timeout error,
 // may happen when the connection was pending for too long,
 // and the remote server or a proxy closed it
 // let's ignore this error and try again
 } else {
 // Any other error - let's show it
 showMessage(response.statusText);
 // Reconnect in one second
 await new Promise(resolve => setTimeout(resolve, 1000));
 }
 }
 
}
subscribe();

Also, 502 is "Bad Gateway" so I think you meant to use 504 "Gateway timeout". I can submit a new PR if your want.

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}