-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Remix crashes with web-fetch 4.3.2/1.8.2 #4993
Comments
This codesandbox URL you shared doesn't seem to work; I've had this challenge when using their newer codesandbox environments 😅 Any way you could copy code into this stackblitz template below?
|
Here's a failing test PR if that helps: #4994 |
Here's a failing test PR if that helps: #4994 |
@jacob-ebey Is the failing test enough? Anything else we can do to help get this fixed? |
I have an |
I'm having the same issue. I just closed #5385 as it's a dupe of this issue.
Interesting that my issue is with fetching from |
@tarngerine Just to confirm, I can see your sandbox properly failing with 4.3.2 |
FYI... Here's the diff between 4.3.1 and 4.3.2 fetch.jsdiff --git a/src/fetch.js b/src/fetch.js
index v4.3.1..v4.3.2 100644
--- a/src/fetch.js
+++ b/src/fetch.js
@@ -346,13 +346,8 @@
}
};
- socket.prependListener('close', onSocketClose);
-
- request.on('abort', () => {
- socket.removeListener('close', onSocketClose);
- });
-
- socket.on('data', buf => {
+ /** @param {Buffer} buf */
+ const onData = buf => {
properLastChunkReceived = Buffer.compare(buf.slice(-5), LAST_CHUNK) === 0;
// Sometimes final 0-length chunk and end of message code are in separate packets
@@ -364,6 +359,14 @@
}
previousChunk = buf;
+ };
+
+ socket.prependListener('close', onSocketClose);
+ socket.on('data', onData);
+
+ request.on('close', () => {
+ socket.removeListener('close', onSocketClose);
+ socket.removeListener('data', onData);
});
});
} request.jsdiff --git a/src/request.js b/src/request.js
index v4.3.1..v4.3.2 100644
--- a/src/request.js
+++ b/src/request.js
@@ -38,6 +38,7 @@
* @property {string} method
* @property {RequestRedirect} redirect
* @property {globalThis.Headers} headers
+ * @property {RequestCredentials} credentials
* @property {URL} parsedURL
* @property {AbortSignal|null} signal
*
@@ -135,6 +136,7 @@
method,
redirect: init.redirect || input.redirect || 'follow',
headers,
+ credentials: init.credentials || 'same-origin',
parsedURL,
signal: signal || null
};
@@ -169,7 +171,7 @@
*/
get credentials() {
- return "same-origin"
+ return this[INTERNALS].credentials
}
/** |
The only issue with reverting back to 4.3.1 is that 4.3.2 was there to fix a memory leak (see remix-run/web-std-io@3b9b384). |
PR created with a possible fix: remix-run/web-std-io#29 Plz review |
I'm surprised that something that crashes Remix isn't getting any attention. |
I'm running into the same issue - it's only with specific websites. Looks like there's a similar problem with the I installed |
FWIW, I also came here to report similar issue. I have also run into it often but it's not consistent. |
Like the other commenters, we have experienced this issue in our Remix project. It was very inconsistent, with identical requests failing consistently one after another one minute, then seemingly working fine the next. I was never able to replicate the issue locally: we had to catch it in production. Replacing our usage of Let me know if I can provide any details of our project that might help debug. I just wanted to bump this issue for visibility. |
The error does not occur if you have |
This is resolved by the version bump in #7026 and should be available when 1.19.2 is released |
This can also be tested in |
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
We've had this issue (intermittently on prod only) and we've updated to |
🤖 Hello there, We just published version Thanks! |
@brophdawg11 was there really something related to this issue in |
🤖 Hello there, We just published version Thanks! |
@ericallam No - I think that's just our bot seeing that there's something new in the prerelease when compared to the latest stable release, so it comments from the comparison of Has |
@brophdawg11 no errors so far 👍 |
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
What version of Remix are you using?
1.8.2
Steps to Reproduce
See sandbox: https://codesandbox.io/p/sandbox/solitary-wave-1l0skr
Failing test PR: #4994
Expected Behavior
web-fetch's fetch should work, without crashing remix, returning a valid HTML result
Actual Behavior
remix crashes, exit code 1
Related issues:
#4737
The text was updated successfully, but these errors were encountered: