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

FetchError: request to http://0.0.0.0:8000/graphql failed, reason: socket hang up #4414

Open
IgnisDa opened this issue Oct 25, 2022 · 21 comments
Labels
bug:unverified feat:fetch Issues related to @remix-run/web-fetch runtime:node

Comments

@IgnisDa
Copy link

IgnisDa commented Oct 25, 2022

What version of Remix are you using?

1.7.3

Steps to Reproduce

I have a graphql server. Making a single request to it in loader or action works fine. But if I have multiple requests:

await graphqlSdk().getStuff();
await graphqlSdk().getStuff();

it fails with the following error:

FetchError: request to http://0.0.0.0:8000/graphql failed, reason: socket hang up
    at ClientRequest.<anonymous> (/workspace/node_modules/.pnpm/@remix-run+web-fetch@4.3.1/node_modules/@remix-run/web-fetch/src/fetch.js:111:11)
    at ClientRequest.emit (node:events:513:28)
    at Socket.socketOnEnd (node:_http_client:512:9)
    at Socket.emit (node:events:525:35)
    at endReadableNT (node:internal/streams/readable:1359:12)
    at processTicksAndRejections (node:internal/process/task_queues:82:21) {
  type: 'system',
  errno: 'ECONNRESET',
  code: 'ECONNRESET',
  erroredSysCall: undefined
}

Expected Behavior

The requests should succeed and my backend should receive 2 requests.

Actual Behavior

The first request succeeds but the second one does not.

@IgnisDa
Copy link
Author

IgnisDa commented Oct 25, 2022

Update: it works fine if I use axios to make the requests.

@machour
Copy link
Collaborator

machour commented Oct 25, 2022

@IgnisDa could you provide a reproduction somewhere? Might be a problem with our web-fetch package

@IgnisDa
Copy link
Author

IgnisDa commented Oct 25, 2022

@machour I am not able to reproduce it anywhere else. Can you give instructions on how to debug the web-fetch package? When I try to add a console.log in the file (that is shown in the error message above), I do not get any output.

@sergiodxa
Copy link
Member

@IgnisDa try to create a repo with a simple Remix app reproducing the error, that way it would be possible to try it and debug it

@IgnisDa
Copy link
Author

IgnisDa commented Oct 25, 2022

@sergiodxa As I said before, I am not able to reproduce it in a simple repository.

The error is occurring in this repository: https://github.com/IgnisDa/codefarem/tree/remix-issue

Could you take a look at the package.json if there are any dependencies that might change the behavior of the web-fetch package?

@zifahm
Copy link

zifahm commented Nov 7, 2022

I have the same error right now using fly in production, works fine in development

@zifahm
Copy link

zifahm commented Nov 7, 2022

can confirm changing from fetch to axios fixes the issue.

I'm quite confused why this would be a problem

@richardbann
Copy link

richardbann commented Nov 14, 2022

Having the same issue here. Will try to put together a minimal example. At the moment it seems like the issue is with web-fetch as simple node:http.request works fine.

@richardbann
Copy link

richardbann commented Nov 14, 2022

Manually setting the http.request option agent to false (in web-fetch) resolves this issue. So it has to do someting with the socket pool... (https://github.com/remix-run/web-std-io/blob/f715b354c8c5b8edc550c5442dec5712705e25e7/packages/fetch/src/request.js#L326)

@machour
Copy link
Collaborator

machour commented Nov 15, 2022

pinging @jacob-ebey since he worked on web-fetch

@richardbann
Copy link

richardbann commented Nov 15, 2022

Ok, this is probably a node 19 thing (nodejs/node#43522). @IgnisDa , @zifahm , can you confirm you are using >= node 19?
The default http agent now uses KeepAlive by default and it is not playing nicely with the manually set connection: close header. (https://github.com/remix-run/web-std-io/blob/f715b354c8c5b8edc550c5442dec5712705e25e7/packages/fetch/src/request.js#L302).
To verify you can set KeepAlive in the fetch call:

resp = await fetch("http://myserver.test", {
  headers: { connection: "keep-alive" },
});

Could you please check @IgnisDa , @zifahm ?
@jacob-ebey , please let me know if I should further investigate.

@zifahm
Copy link

zifahm commented Nov 15, 2022

@richardbann yup is using ENV NODE_VERSION=19.1.0

@zifahm
Copy link

zifahm commented Nov 15, 2022

hmm was using node:current in the docker container. now switched to node:lts-slim

@IgnisDa
Copy link
Author

IgnisDa commented Nov 15, 2022

Can confirm headers: { connection: "keep-alive" } fixes it.

IgnisDa added a commit to IgnisDa/codefarem that referenced this issue Nov 15, 2022
Using the solution at remix-run/remix#4414 (comment).
However, it would be better if we do not have to write a custom fetch
function at all.
IgnisDa added a commit to IgnisDa/codefarem that referenced this issue Nov 25, 2022
* build(rust): optimize binaries for size

* style(moon): remove comments from file

* build: pin dependencies

* chore(react-ui): add react imports to components

* ci: use `--all` flag to test all projects

* ci: disable pruning of dependencies when running task

* build: install moonrepo-cli as dependency

* feat(main-db): remove empty test case unit

* build: declare chrono as a workspace dependency

* build(website): add routes-gen dependencies

Also remove remix-routes

* feat(website): use routes-gen for route generation

* feat(main-db): new library for edgeql string conversion

* build(website): add uuid package as dependency

* feat(orchestrator): add resolver to get question details

* feat(website): display question details on question/:id page

* chore: delete useless files

* ci: remove log level from check command

* ci(devcontainer): install moon directly from source

* ci(main-db): make `test` depend on `generate` task

* feat(website): remove all tailwind classes

* feat(react-ui): remove button and input components

* feat(website): add logic handling to authentication routes

* ci: add typecheck target to all project

* build(orchestrator): update edgedb versions

* feat(faker): add library to generate fake data

* ci: exclude build task from projects

* ci(eslint): add generated files to ignoring eslint

* ci(faker): configure watch command

* refactor(main-db): handle user data creation from library

* refactor(main-db): use fake random generator from library

* ci(faker): add  moon configuration

* ci: move dep to workspace root

* build: remove moon dev dep

* ci(main-db): mark generate as local task

* feat(website): use pre-populate with fake data in dev mode

* feat(website): replace using `axios` with `fetch`

Using the solution at remix-run/remix#4414 (comment).
However, it would be better if we do not have to write a custom fetch
function at all.

* fix(website): graphql fetch function

* fix(website): throw error when unable to get user details

* fix(website/auth): broken login flow because of error handling

* fix(website): use correct btn label

* ci: use correct moon command to check validity

* feat:  use slug for creating questions

* ci(generated): do not emit declarations

* fix(learning): return correct error when question not exists

* feat(website): handle edge cases while creating questions

Also show correct details non existent question
is requested.

* refactor(website): reference projects directly

Using the guide at
https://moonrepo.dev/docs/guides/javascript/typescript-project-refs#using-paths-aliases.

* ci(website): enable typechecking for project

* feat(generated): replace gql zeus with codegen

* feat(website): migrate to using graphql codegen

* ci: configure tasks for workspace

* refactor(main-db): use package source code directly

* ci(main-db): try to add workspaces support

* fix(website): add connection header to all reqs

Apparently due to changes in how node19 works as described in
https://nodejs.org/en/blog/announcements/v19-release-announce/#http-s-1-1-keepalive-by-default,
we need to add this header to fix the error: `FetchError: request
to http://0.0.0.0:8000/graphql failed, reason: socket hang up`.

* fix(website/playground): add color to loading icon

* build(orchestrator): add comrak deps

* feat(orchestrator): return rendered markdown from problem

* feat(generated): add additional attributes to fetch from backend

* feat(website): add page to solve questions

This is just a skeleton. It does not work yet because it does not take into account the test case
that is displayed on the screen.

* fix(website): redirect to cpp page after question creation

* ci: route outDir to project caches

* fix(website): add correct types to data

* test(main-db): try to fix jest config

* ci(moon): remove useless commands

Also configure the website's typecheck command
to get the correct input files.

* test(main-db): fix broken jest config

* feat(website): change import path for fake module

* ci(vscode): add word to grammar

* feat: add handling for testing questions

* feat: rewrite how test cases are stored

Instead of creating separate models for each test case data
type, we will store them all in a string and let the data
decide how to work with it. However, this method will result
in us losing the ability to do strict output comparisons (we have
implemented only  string based output correctness checking). I
do not think I am willing to give up such a big feature for a little
ease of db schema. However I will keep this in commit history
so that I can reference it later down the line.

* Revert "feat: rewrite how test cases are stored"

This reverts commit c9e20c3.

* ci(devcontainer): install edgedb for non-root user
@eewang
Copy link

eewang commented Dec 21, 2022

Same here - I ran across the same FetchError triggered in the web-fetch package, though adding { connection: "keep-alive" } to my headers object fixed the issue. 👍

@brophdawg11 brophdawg11 added runtime:node feat:fetch Issues related to @remix-run/web-fetch labels May 5, 2023
@Jarrku
Copy link

Jarrku commented Sep 19, 2023

Ran into this after upgrading our remix project to node 20 as well. Can also confirm manually setting the connection header resolved the issue for us. The error triggered when we made multiple requests to the same API server in a single loader.

@gaetan-altech
Copy link

gaetan-altech commented Feb 5, 2024

I've got the same kind of problem with a node 21 project, remix 2.5.1. The 'connection' workaround seems to work.

@hunt3r
Copy link

hunt3r commented Feb 13, 2024

Same here - I ran across the same FetchError triggered in the web-fetch package, though adding { connection: "keep-alive" } to my headers object fixed the issue. 👍

Thanks, this one was driving me crazy.

@JesseFarebro
Copy link

Are there any leads on solving the root of this issue? I'm using a 3rd party API where I can't add additional headers to fix this issue.

@IgnisDa
Copy link
Author

IgnisDa commented Apr 2, 2024

@JesseFarebro The headers need to be set on your side when making the http request. Why can't you set the header?

@JesseFarebro
Copy link

@IgnisDa I'm using a library that ends up calling fetch down the call stack. There is no options to pass through headers to fetch. The path of least resistance is probably to just wholesale copy some of their code for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:unverified feat:fetch Issues related to @remix-run/web-fetch runtime:node
Projects
None yet
Development

No branches or pull requests