Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eventSource.onerror triggered before page reload #23

Open
apostrofix opened this issue Sep 10, 2024 · 5 comments
Open

eventSource.onerror triggered before page reload #23

apostrofix opened this issue Sep 10, 2024 · 5 comments

Comments

@apostrofix
Copy link

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?

@lukas-reining
Copy link
Owner

lukas-reining commented Sep 10, 2024

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
Copy link
Owner

lukas-reining commented Sep 10, 2024

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:

if (typeof error === 'object' && error?.name === 'AbortError') {

Would be happy if you could provide me with the error details @ivobelyasin :)

@apostrofix
Copy link
Author

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.

@lukas-reining
Copy link
Owner

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: nodejs/node#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:

this.logger?.warn(msg, error ?? '');

@lukas-reining
Copy link
Owner

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants