Errors not propogated to the user

[lukebelbina]

  function connect() {
    if (request) {
      return
    }

    readyState = CONNECTING
    controller = new AbortController()
    request = fetch(url, getRequestOptions())
      .then(onFetchResponse)
      .catch((err: Error & {type: string}) => {
        request = null

        // We expect abort errors when the user manually calls `close()` - ignore those
        if (err.name === 'AbortError' || err.type === 'aborted') {
          return
        }

        scheduleReconnect()
      })
  }

https://github.com/rexxars/eventsource-client/blob/d8244e895b0c47bcb15fc4f8dd1f098ed42daa57/src/client.ts#L80C2-L99C4

My understanding based on the code is when there is an error connecting there is no way to know. For my use case I'd like to display something to the user / have some logic to deal with errors like this.

I am planning on using this library - do you want a PR to add an onConnectionError or some other event?

Additionally, I'd like to update the reconnect logic to support some sort of back off strategy, do you want that in the library also?

[punkpeye]

@lukebelbina Did you figure this out?

[lukebelbina]

@lukebelbina Did you figure this out?

I ended up pulling it down locally and making a couple changes, mainly

Adding

  onConnectionError?: (error: Error, statusCode?: number) => void;

function connect() {
    if (request) {
      return;
    }

    readyState = CONNECTING;
    controller = new AbortController();
    request = fetch(url, getRequestOptions())
      .then(onFetchResponse)
      .catch((err: Error & { type: string }) => {
        request = null;

        // We expect abort errors when the user manually calls `close()` - ignore those
        if (err.name === 'AbortError' || err.type === 'aborted') {
          return;
        }

        onConnectionError(err);

        scheduleReconnect();
      });
  }

And throwing on non 200 status in onFetchResponse

async function onFetchResponse(response: FetchLikeResponse) {
    onConnect();
    parser.reset();

    const { body, redirected, status } = response;

    // HTTP 204 means "close the connection, no more data will be sent"
    if (status === 204) {
      onDisconnect();
      close();
      return;
    }

    if (status >= 400) {
      // todo better and more informative errors for the user
      // todo parse error body for more info
      throw new Error(`Unable to connect with HTTP status ${status}`);
    }

I was waiting to hear back from the maintainer if it was worth doing a PR. I'll give it until after the holidays and if I don't hear anything back I'll fork / submit a PR.

[punkpeye]

Thank you

[rexxars]

Definitely something that should be added. I started some work on this a while back but didn't have time to finish it up.
The status code callback should be an easy add, but I also want to see if I can catch situations like if the API responds with non-SSE data (but gives a 200 OK).

[gregsadetsky]

heya @rexxars thanks a ton for this library! I just ran into this issue - upon encountering a 401, the library reconnects

would you be open to a PR for this? or sharing the code / ideas you had for the fix so that I could try my hand at it?

thanks a lot!

[punkpeye]

@lukebelbina Would you be open to submitting the PR?

[punkpeye]

At the moment, if fetch throws an error, it seems like the iterator hangs indefinitely.

it.only('should handle errors', async () => {
  const es = createEventSource({
    fetch: async () => {
      throw new Error('test error');
    },
    url: 'https://example.com/sse',
  });

  const messages = [];

  for await (const message of es) {}

  // We never get here
});

[OliverJAsh]

If I understand correctly, currently it will retry forever. I think it would make sense to retry 3/4/5 times before returning an error somehow e.g. via an onError callback, similar to React Query.