eventsource icon indicating copy to clipboard operation
eventsource copied to clipboard

eventSource.onerror triggered before page reload

Open ivo-enmacc opened this issue 1 year ago • 7 comments

Hello, first of all, thank you for providing this package! It works really well and it's easy to use!

I am just wondering, after a successful connection, if I reload the page, the onerror event gets triggered. I would like to have the listener for the error events so that I can send errors to my monitoring solutions, however this one doesn't exactly seem like an error but rather that the connection is just dropped by the browser probably (because of the page reload).

As a workaround I added:

window.onbeforeunload = () => {
  // disconnect
  this.eventSource.close();
  this.eventSource = null;
};

This works fine, however I am not the biggest fan of the onbeforeunload event and I am just wondering if that's the best approach. Did anybody else run into this scenario?

ivo-enmacc avatar Sep 10 '24 08:09 ivo-enmacc

Hey @ivobelyasin thank you for the nice feedback! :) I see the problem and I guess your interpretation is correct. Do you see the actual error there? Can you post it here?

I am not sure if we could add the window.addEventListener("beforeunload", (event) => {this.close()}); to the library. Probably this would solve it, but I am not sure if this would lead to problems in edge cases. I will try it out and research a bit, maybe you @ivobelyasin or someone else has thoughts to that.

The first thing that comes to my mind is what happens if any other part of the app implements something like:


window.onbeforeunload = (event) => {
  const shouldBlockClose = ...
  if(shouldBlockClose){
    event.preventDefault();
  }
};

and it shouldBlockClose. In this case we would close the event source but closing the window might be broken. There are also several issues described here, so probably we can not use it for this use case: https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event#usage_notes

I think we could evaluate the https://developer.chrome.com/docs/web-platform/page-lifecycle-api and maybe use freeze and resume but I will have to investigate a bit.

lukas-reining avatar Sep 10 '24 14:09 lukas-reining

Actually I can not reproduce it locally with a really simple app. For me an AbortError is thrown, which is expected, but caught by the following line: https://github.com/lukas-reining/eventsource/blob/daf7843ab2c9aed43f00556a6c0f3ca8a386cc81/src/eventsource.ts#L186 Would be happy if you could provide me with the error details @ivobelyasin :)

lukas-reining avatar Sep 10 '24 14:09 lukas-reining

Thank you for the quick reply @lukas-reining ! That's a very good point, listening to the onbeforeunload event opens possibilities for edge cases.

I currently have this as error handler:

this.eventSource.onerror = (error) => {
  console.log(error);

  if (typeof error === 'object' && (error as unknown as Error)?.name === 'AbortError') {
    return;
  }

  this.logError(error);
};

I just added the check for AbortError, however the error I get looks like this:

Screenshot 2024-09-10 at 18 05 28

The problem is that the browser doesn't allow me to expand it and see the details. I'm using the preserve log option but still...It seems like it's maybe not a proper error, as the name property is undefined if I try to log it.

ivo-enmacc avatar Sep 10 '24 16:09 ivo-enmacc

I currently have this as error handler:

this.eventSource.onerror = (error) => {
  console.log(error);

  if (typeof error === 'object' && (error as unknown as Error)?.name === 'AbortError') {
    return;
  }

  this.logError(error);
};

I just added the check for AbortError, however the error I get looks like this:

Screenshot 2024-09-10 at 18 05 28

The problem is that the browser doesn't allow me to expand it and see the details. I'm using the preserve log option but still...It seems like it's maybe not a proper error, as the name property is undefined if I try to log it.

The problem is that the event source is not returning an ErrorEvent here, but a normal Event. Node does not have the DOM ErrorEvent, this is why I opted for a normal event here, which is fine to the spec as seen here: https://html.spec.whatwg.org/multipage/server-sent-events.html#handler-eventsource-onerror This is the issue in the Node repo: https://github.com/nodejs/node/issues/47587.

So I think your check can not work. Have you enabled logging and see a warning log? I would expect it to be visible because of: https://github.com/lukas-reining/eventsource/blob/daf7843ab2c9aed43f00556a6c0f3ca8a386cc81/src/eventsource.ts#L212

lukas-reining avatar Sep 11 '24 19:09 lukas-reining

If you are getting Reconnecting because EventSource connection closed I would have an idea how calling the error handler can be prevented. Would be great to know what you see @ivobelyasin :)

lukas-reining avatar Sep 11 '24 19:09 lukas-reining

Hey, a little comment, not very related to the problem described, but could be helpful anyway:

The problem is that the browser doesn't allow me to expand it and see the details. I'm using the preserve log option but still...It seems like it's maybe not a proper error, as the name property is undefined if I try to log it.

@apostrofix That's because of the page reload. The object ceases to exist, so you can no longer access it, what you see & can access is the output kept in memory.

Do something like that:

for (let key in e) {
  console.log(key, e[key])
}

It should preserve more visible data. Repeat this for nested objects if needed.

dzek69 avatar Oct 20 '24 17:10 dzek69

Thanks @dzek69! @apostrofix are you still interested in getting this solved? If you are, I would be grateful if you could provide me the requested info.

lukas-reining avatar Dec 15 '24 13:12 lukas-reining