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

Handle aborted requests #102

Closed
carbonrobot opened this issue Oct 3, 2023 · 5 comments
Closed

Handle aborted requests #102

carbonrobot opened this issue Oct 3, 2023 · 5 comments
Labels
@envyjs/node Node package @envyjs/web Web package @envyjs/webui Browser package
Milestone

Comments

@carbonrobot
Copy link
Contributor

Requests using fetch have the capability to be aborted (using an AbortController). This is used in urql for example, to cancel an in-flight GQL request when a fresher one has started.

Currently in Envy, we don't get notified that the request was aborted, so the trace shows as incomplete. It would be nice if we were able to identify when a request is aborted and to send something across the web socket to Envy so that Envy can flag the request as aborted.

@carbonrobot
Copy link
Contributor Author

I think we need a two prong approach, since not all network packages handle aborted well, and we could also just have a disconnect.

  1. Core/Web: Add a http?.status property and send an event when a request is aborted
  2. WebUI: After a set timeout, cleanup any requests that didn't receive a response and set the UX to show something to the user in those cases

@carbonrobot
Copy link
Contributor Author

@kgpax Any thoughts on the second bullet point above?

@carbonrobot carbonrobot added @envyjs/webui Browser package @envyjs/web Web package @envyjs/node Node package labels Oct 4, 2023
@carbonrobot carbonrobot added this to the Release Zero milestone Oct 4, 2023
@kgpax
Copy link
Member

kgpax commented Oct 4, 2023

Sounds good to me. You thinking that we remove those traces from the list (might get "jumpy") or just update them as "timed out"?

@carbonrobot
Copy link
Contributor Author

Maybe just mark them so we dont have scroll issues?

@carbonrobot
Copy link
Contributor Author

Split issue into #124 #125 #126 #127

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@envyjs/node Node package @envyjs/web Web package @envyjs/webui Browser package
Projects
None yet
Development

No branches or pull requests

2 participants