Skip push requests when the browser is offline#677
Conversation
|
✔️ All good! |
ssh000
force-pushed
the
fix/skip-requests-when-browser-is-offline
branch
from
d7fd833 to
9119f63
Compare
Reject pushes in `PushApi#push` when `navigator.onLine === false` so no fetches are made without connectivity. Existing catch paths queue the span and let the dispatcher keep retrying. Fixes: appsignal/support#338
ssh000
force-pushed
the
fix/skip-requests-when-browser-is-offline
branch
from
9119f63 to
3b82c3c
Compare
There was a problem hiding this comment.
We should take a look at the "queue" mechanism for the integration, which should attempt to retry sending these at a later point. I recall taking a look at it at some point and suspecting it to not actually work, which would mean whatever was being pushed is lost forever, not retried at a later time.
That said, that's a pre-existing issue, and as such this PR is an improvement over status quo.
| */ | ||
| public async push(span: Span): Promise<Span> { | ||
| if (isOffline()) { | ||
| return Promise.reject() |
There was a problem hiding this comment.
Does something catch this rejection? An unhandled promise rejection would trigger a plugin-window-events loop in the same way that uncaught exceptions do, as described in https://github.com/appsignal/support/issues/338#issuecomment-2488668536
There was a problem hiding this comment.
There is a catch that swallows the error, I also tested it
|
This is a message from the daily scheduled checks. |
Yes I noticed that queue is drained even if it cannot send errors and I also noticed that backoff factor makes next retries too far away in time so when you recovered from an error or becoming online you will not send queue in the near feature. I didn't want to mix it all together, but if you want me I can do this here? |
2 participants
Prevent sending a request when a browser is offline
Fixes: appsignal/support#338