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

Fetch resolves before response has been received; response is undefined #13187

Closed
3 tasks done
andreasphil opened this issue Aug 2, 2024 · 14 comments · Fixed by #13202
Closed
3 tasks done

Fetch resolves before response has been received; response is undefined #13187

andreasphil opened this issue Aug 2, 2024 · 14 comments · Fixed by #13202
Assignees
Labels
Package: vue Issues related to the Sentry Vue SDK Type: Bug

Comments

@andreasphil
Copy link

andreasphil commented Aug 2, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/vue

SDK Version

8.22.0

Framework Version

Vue 3.4.35

Link to Sentry event

No response

Reproduction Example/SDK Setup

Unfortunately I don't have a reproduction, but this is our setup:

(Note that I debugged useFetch and it seems to not be a useFetch issue, see Steps to Reproduce below)

Steps to Reproduce

Summary: for some long-running (a few seconds, but <10s), fetch resolves before the actual request has finished. This leads to the fetch response being undefined. Anything accessing properties of the response then crashes. Reverting to @sentry/vue v8.20.0 fixes the issue.

Example:

fetch("some-long-running-request").then((response) => {
  // Now response is sometimes undefined, so the following would lead to a TypeError:
  if (response.status === 400) {
    // ...
  }
})

Looking at the network dev tools and server logs, I can verify that the request is fulfilled successfully. However it seems that fetch resolves before the request has finished, with undefined as the value for the response.

Possibly related:

Expected Result

Fetch should not resolve before the request is finished, and response should not be undefined.

Actual Result

Fetch resolves before the request has finished, response is undefined, and accessing properties of the response causes errors.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 2, 2024
@github-actions github-actions bot added the Package: vue Issues related to the Sentry Vue SDK label Aug 2, 2024
@chargome
Copy link
Member

chargome commented Aug 2, 2024

Hey @andreasphil thanks for reaching out. The PR you mentioned should not affect your original fetch call, as we just try to clone the response and resolve it in parallel.

Does the endpoint you are calling make use of SSE?

Anyway, I'll try to verify your issue and get back to you.

@andreasphil
Copy link
Author

@chargome Thanks for the quick reply!

Yes, it might be unrelated. I'm not familiar with the Sentry code base, just did some quick scanning of the changelogs since v8.20 to see if anything catches my attention.

We're not using SSE.

Thanks for looking into it and let me know if you need more information.

@esetnik
Copy link

esetnik commented Aug 2, 2024

We have been hit by the same issue and are rolling back to 8.20.0. It looks to be related to #12723

@chargome
Copy link
Member

chargome commented Aug 2, 2024

@esetnik are you also using the @sentry/vue SDK?

@esetnik
Copy link

esetnik commented Aug 2, 2024

@esetnik are you also using the @sentry/vue SDK?

No we are using @sentry/react

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 2, 2024
@chargome
Copy link
Member

chargome commented Aug 2, 2024

@andreasphil I couldn't reproduce this issue with a long running browser fetch request from vue (15s) - everything still worked with 8.22. Will try to see if useFetch has any impact. Do you have any details on how the request is handled on the server side?

@esetnik Could you also provide more details on your setup, maybe we can pin down the issue faster this way.

A small reproducible example would be really helpful for fixing this issue.

@andreasphil
Copy link
Author

@chargome On the server side we're using a pretty standard Java Spring Boot REST API. This is one of the controllers for the requests that were starting to reliably cause issues after the SDK update.

FWIW all our code is public in the repo I linked (frontend + backend). It's not a small codebase though so I don't expect you to read all of it, but maybe it still helps.

Unfortunately I'm not sure if I will find the time to create a reproducible example but I'll see what I can do next week.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 2, 2024
@chargome
Copy link
Member

chargome commented Aug 2, 2024

@andreasphil thanks, let's see if we can reproduce it somehow.

@esetnik
Copy link

esetnik commented Aug 2, 2024

I believe it's related to aborted fetch requests. We use an AbortController to abort the requests and seem to be seeing this only on endpoints where we actually frequently abort fetch requests.

@AbhiPrasad
Copy link
Member

@esetnik great suggestion on the aborted request - that seems to be it

opened #13202 to fix this, we'll get a release out on monday after I add (several) e2e tests 😄

@andreasphil
Copy link
Author

andreasphil commented Aug 2, 2024

@esetnik good catch, I was about to say something similar. I did some debugging with our app including the Sentry SDK, here are some findings:

  • In our case too the two places in the frontend where we saw the issue make use of AbortControllers. Those happen also to be our longest running requests so I might've gotten confused there (sorry!)
  • Specifically, useFetch tends to use AbortControllers frequently because it re-fetches when relevant parameters such as the URL change, aborting pending requests.
  • Setting traceFetch to false solves the issue even in @sentry/vue 8.22.0, so perhaps it is caused by something that happens in the instrumentation of outgoing requests or one of the handlers that get attached there
  • We specifically treat AbortErrors in our fetch hooks and this is not called anymore because aborted requests error due to the response being undefined
  • From what I could see in the debugger, the AbortError does appear in various places where Sentry handles the fetch result, but somehow then fetch is still resolved

This is where I'm going to stop for today but hope that helps. And again, thanks for looking into it!

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 2, 2024
@AbhiPrasad
Copy link
Member

thanks for validating @andreasphil

and to everyone who runs into this, sorry for the trouble, we're on this to fix! Appreciate all the help.

@lforst
Copy link
Member

lforst commented Aug 5, 2024

Hi all, we just released version 8.23.0 of the SDK which includes a fix for this bug. Feel free to try it out and let us know if it resolved the issue for you. Thanks!

andreasphil added a commit to digitalservicebund/ris-norms that referenced this issue Aug 6, 2024
Version 8.22 caused issues for us, see:

getsentry/sentry-javascript#13187

This should be fixed now, so upgrade to 8.23 and allow automatic updating
again.

RISDEV-0000
@andreasphil
Copy link
Author

@lforst I can confirm the issue is fixed for us. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: vue Issues related to the Sentry Vue SDK Type: Bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants