sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

fix(fetch): properly release fetch during long-lived stream handling

Open Lei-k opened this issue 1 year ago • 7 comments

Hi, I saw that the trackFetchStreamPerformance option was introduced in PR #13951 to bypass this issue, and Issue #13950 was closed as a result. However, this hasn't fully resolved the problem, so I attempted to address it in this PR.

Here is my modified test result after applying the changes:

截圖 2024-10-13 上午2 39 23

Lei-k avatar Oct 12 '24 18:10 Lei-k

@Lei-k we have not released #13951 yet, can you specify why this does not resolve the issue?

chargome avatar Oct 14 '24 08:10 chargome

@chargome From what I understand, in #13951 only adds the trackFetchStreamPerformance option and sets it to false by default to bypass the issue. However, when this option is enabled, the problem still persists.

Lei-k avatar Oct 14 '24 08:10 Lei-k

I saw some errors in the CI. I'll deal with them later

Lei-k avatar Oct 16 '24 14:10 Lei-k

It seems that there are no further errors related to this PR. Can you please help confirm this, @chargome .

Lei-k avatar Oct 18 '24 07:10 Lei-k

擷取_2024_10_18_17_33_23_474 擷取_2024_10_18_17_35_01_322 擷取_2024_10_18_17_35_06_654

@chargome

Response was already in the code before, but now it's triggering a TS2304 error. Do you have any idea why this is happening?

It looks like only the TS2304 error and the file size check remain to be solved now.

Lei-k avatar Oct 18 '24 09:10 Lei-k

@Lei-k not sure, but the same error is also crashing node-otel-sdk-node and node-otel-custom-sampler tests

chargome avatar Oct 18 '24 10:10 chargome

@chargome

All tests have passed on my side. Please check, thanks!

Lei-k avatar Oct 20 '24 07:10 Lei-k

Hi, @chargome

There’s one remaining bundle size check that failed due to the size limit. Could you assist me in adjusting the size, or do you have any suggestions for resolving this?

Lei-k avatar Oct 21 '24 11:10 Lei-k

Closing this one for now, we might need to re-visit the entire fetch instrumentation for streams

chargome avatar May 20 '25 09:05 chargome

Hi @chargome, Sorry, I just realized that my earlier comments were not actually submitted. I’ll sync with the latest branch and check the situation, and if needed, I’ll open a new PR to address it.

Lei-k avatar Aug 26 '25 19:08 Lei-k