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

(svelte) - "Failed to fetch" error not caught in CombinedError #2234

Closed
max-las opened this issue Feb 2, 2022 · 6 comments · Fixed by #2287
Closed

(svelte) - "Failed to fetch" error not caught in CombinedError #2234

max-las opened this issue Feb 2, 2022 · 6 comments · Fixed by #2287
Labels
bug 🐛 Oh no! A bug or unintented behaviour.

Comments

@max-las
Copy link

max-las commented Feb 2, 2022

I'm using Svelte bindings. When my API in down and I try to query it, the error Uncaught (in promise) Error: Failed to fetch is thrown, and it is not reflected in $myOperationStore.error. Instead, $myOperationStore.fetching remains true and $myOperationStore.error remains undefined. So I'm not sure how to handle that case.

Here is my code :

<script>
  import { operationStore, query } from '@urql/svelte';

  let username = null;
  let usernameError = null;
  let usernameSuccess = null;
  let usernameWarning = null;

  function resetUsername() {
    usernameSuccess = null;
    usernameError = null;
    usernameWarning = null;
  }

  const caniuse = operationStore(
    `
    query ($username: String!) {
      caniuse(username: $username)
    }`,
    { username },
    { pause: true }
  );

  query(caniuse);

  function queryCaniuse(username) {
    $caniuse.variables.username = username;
    $caniuse.reexecute({ pause: false });
  }

  $: {
    if(username){
      queryCaniuse(username);
    }else{
      resetUsername();
    }
  }

  $: {
    resetUsername();

    if (!$caniuse.context.pause) {
      if ($caniuse.fetching) {
          usernameWarning = "Checking username availability...";
      } else {
        if (!$caniuse.error) {
          if ($caniuse.data.caniuse) {
            usernameSuccess = "Username available";
          } else {
            usernameError = "Username unavaible";
          }
        } else {
          usernameError = "Something went wrong";
        }
      }
    }
  }
</script>

urql version:
@urql/svelte 1.3.3

Steps to reproduce

  1. Setup an operationStore holding a query
  2. Run the query while the API server is down
  3. Observe the presence of Uncaught (in promise) Error: Failed to fetch in the console
  4. Observe the value of the properties error and fetching of the operationStore

Expected behavior

I suppose failing to fetch should result in a networkError held in the CombinedError returned by the operationStore.

Actual behavior

Error is ignored by urql and just happens with no way to handle.

@max-las max-las added the bug 🐛 Oh no! A bug or unintented behaviour. label Feb 2, 2022
@kitten kitten changed the title "Failed to fetch" error not caught in CombinedError (svelte) - "Failed to fetch" error not caught in CombinedError Feb 2, 2022
@orangecms
Copy link
Contributor

Ran into this with urql (React) as well. I have found that @urql/core ends up treating this as a TypeError using a local build, thrown in fetchSource.ts. I'll see what I can figure out.

@orangecms
Copy link
Contributor

orangecms commented Feb 9, 2022

Simple: If not passing a custom fetcher, urql defaults to window.fetch.
To reproduce the issue, just do this in dev tools:

fetch("http://wjejfufiiendj").catch(console.error)

Chromium 98 here logs TypeError: Failed to fetch.
How should urql behave? Pass through TypeError? (why do they even call it that? Time to read Chromium/v8 docs, MDN ...) 😅

Introduced here:
20b7951

The changelog says:

Fix error bubbling, when an error happened in the exchange-pipeline we would treat it as a GraphQL-error

@orangecms
Copy link
Contributor

https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#checking_that_the_fetch_was_successful

A fetch() promise will reject with a TypeError when a network error is encountered or CORS is misconfigured on the server-side, although this usually means permission issues or similar — a 404 does not constitute a network error, for example. An accurate check for a successful fetch() would include checking that the promise resolved, then checking that the Response.ok property has a value of true.

for reference

@kitten
Copy link
Member

kitten commented Feb 16, 2022

I suppose this is a regression from #2210, since it 100% sounds like it, so I think I can preemptively confirm that this is a problem

@dvicory
Copy link

dvicory commented Feb 16, 2022

I can confirm we're seeing this behavior as well when the backend server is down. This caused us some interesting issues where dedup exchange would filter out reexecutions, since it believed queries were still in flight since it never received an OperationResult. There was the same uncaught Uncaught (in promise) Error: NetworkError when attempting to fetch resource.

dvicory added a commit to Mookiies/crdt-exploration-frontend that referenced this issue Feb 16, 2022
…t will never show optimistic results

Also use a cache-only requestPolicy to prevent refetching queries every time. It seems that cache-first
gets upgraded to cache-and-network by graphcache upon receiving a mutation result. It's not known why
it decides to do this right now.

This was caused by the dedup exchange filtering out the reexecuted queries, believe them to
be extraneous. It would never see the OperationResult coming back since server offline causes uncaught
errors to be thrown from the fetch exchange. That in turn causes no errored results to come back.

The easy fix is to simply place queries on the rebound channel, rather than reexecuting them
through the client. Whether or not this is "correct" is hard to say.

The underlying fix should be tracked via urql-graphql/urql#2234
@JoviDeCroock
Copy link
Collaborator

This should be fixed in @urql/core 2.4.2 by the #2287 PR

@JoviDeCroock JoviDeCroock linked a pull request Feb 18, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Oh no! A bug or unintented behaviour.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants