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

[data] useQuery hangs forever instead of returning network error on bad URL #78

Open
Coachonko opened this issue Jun 25, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@Coachonko
Copy link

Coachonko commented Jun 25, 2024

I am trying to use data.
~~It seems like useQuery hangs until timeout if the server api object contains a fetch call. ~~
The API endpoint receives no requests, as if fetch was not called.

// On the server
const api = {
    // retrieveAdmin: () => fetch(`${config.apiUrl}/euxenite/auth`) // this hangs the server
    retrieveAdmin: () => fetch(`https://google.com`) // this works
    // retrieveAdmin: () => new Promise((resolve) => { // this works fine
    //  setTimeout(() => {
    //    resolve(testObject)
    //  }, 1000)
    // })
  }
}

// In the component
const res = useQuery('adminData', api.retrieveAdmin)

I am on Bun, Bun does support fetch, and it shouldn't be the cause of the issue. Is the server implementation of data supposed to work differently?

It looks like the problem is not fetch. The api url I provided was bad, but I did not expect useQuery to hang forever, I expected a network error to be returned

I noticed that the server waits the 1s timeout I set before it returns the response to the browser (of course).
In testing, with the api on the same machine, there shouldn't be much wait time. But in production, what happens if a data source doesn't respond quickly enough?
Because useQuery uses the api object that has the same type on server and browser, is it impossible to useQuery on browser only for some api properties? Or maybe a way to use useQuery on the browser only, despite a SSR setup?

@Coachonko Coachonko changed the title [data] Fetch on the server doesn't seem to work [data] useQuery hangs forever instead of returning network error on bad URL Jun 25, 2024
@atellmer
Copy link
Owner

atellmer commented Jun 26, 2024

It looks like the problem is not fetch. The api url I provided was bad, but I did not expect useQuery to hang forever, I expected a network error to be returned

This hook uses try catch and await. If the promise (in your case the fetch call) throws an error, then it is intercepted by the catch block. useQuery doesn't know anything about the network or network errors; it works with pure promises. I'm not sure, but your fetch call doesn't seem to throw an error. Try this:

try {
  await fetch(`${config.apiUrl}/euxenite/auth`)
} catch(error) {
  console.log(error) // is there error in console?
}

In testing, with the api on the same machine, there shouldn't be much wait time. But in production, what happens if a data source doesn't respond quickly enough?

Since we are just using promises, Dark will wait until the promise resolves. It doesn't matter how long it takes. This render will wait, and all other renders from other users (I mean SSR) will be queued.

Because useQuery uses the api object that has the same type on server and browser, is it impossible to useQuery on browser only for some api properties? Or maybe a way to use useQuery on the browser only, despite a SSR setup?

If you use SSR, you must provide identical versions of the content in order for the hydration to be successful. And to do this, you must generate identical content on both the server and the client. Therefore, the api must be fully implemented for the client and the server.

In theory, it would be possible not to load data on the server, but to send, say, a spinner and continue loading data on the client via useEffect. However, this approach loses the whole point of SSR, in which the user or search robot should receive all the content.

If you want to load some data only on the client, then it might be better to use useEffect.

@Coachonko
Copy link
Author

Coachonko commented Jun 26, 2024

I'm not sure, but your fetch call doesn't seem to throw an error.

Indeed, it doesn't. It seems like the fetch call just hangs and doesn't return anything. This may be out of scope then.

I think I will need to wrap all the fetch calls in some timeout-management to prevent my server to accumulate unresolved+unrejected+unerrored promises.
I could give an AbortController.signal

export const api = {
  retrieveAdmin: ({ signal }) => fetch(`${config.apiUrl}/euxenite/auth`, { signal })
}
// Then in the component
  const controller = new AbortController() // make persist with state?
  const res = useQuery('adminData', api.retrieveAdmin, { variables: { signal: controller.signal } })
  const handleClick = () => controller.abort() // or setTimeout

Seems a little messy, it would be convenient to have something like Tanstack's queryClient.cancelQueries({ queryKey })

If you want to load some data only on the client, then it might be better to use useEffect.

I see, I do want to take advantage of the caching and have all data in the same cache though


I think the util function stringify in the data package should be moved to core and exported, and used throughout, to shrink bundle sizes even further

@atellmer
Copy link
Owner

atellmer commented Jun 26, 2024

Seems a little messy, it would be convenient to have something like Tanstack's queryClient.cancelQueries({ queryKey })

I think this makes sense.

But as a workaround, you can make useAbort hook and pass its result to useQuery.

const api = {
  fetchTodo: (signal?: AbortSignal) => () => fetch('https://jsonplaceholder.typicode.com/todos/1', { signal }),
};

function useAbort<T extends (...args: Array<any>) => Promise<unknown>>(fn: (signal: AbortSignal) => T) {
  return useMemo(() => {
    const ctrl = new AbortController();
    const method = fn(ctrl.signal);

    return [method, (reason: string) => ctrl.abort(reason)] as [T, (reason: string) => void];
  }, []);
}

const App = component(() => {
  const [fetchTodo, abort] = useAbort(api.fetchTodo);
  // put fetchTodo in useQuery

  useEffect(() => {
    fetchTodo()
      .then(res => res.json())
      .then(data => console.log(data))
      .catch(error => console.log(error));
    abort('abort request'); // test abort
  });

  return null;
});

But you need to think about what to do if you want to run the api call a second time.

@Coachonko
Copy link
Author

Coachonko commented Jun 26, 2024

Today is a bad day, I can't even get my browser to stop deleting cookies on page refresh, for some reason.

I am trying to follow the docs as closely as possible.

const Home = component(() => {
  const api = useApi()
  const { isFetching, data, error } = useQuery('adminData', api.retrieveAdmin)

  if (isFetching && !data) return <div>Loading...</div>
  if (error) return <div>{error}</div>
  console.log(data.status) // 401 on server, undefined on browser
  return (
    <>
      {data.status} {/* Inconsistent element for hydration! */}
    </>
  )
})

What could be wrong here?
The data object seems to be exactly what I would get when a fetch call resolves.

Suppose I am getting a text or json body, I would need to parse the body to json, but I can't just const actualData = await data.json() in this component. And if I parse it this way

const Home = component(() => {
  const api = useApi()
  const { isFetching, data, error } = useQuery('adminData', api.retrieveAdmin)

  if (isFetching && !data) return <div>Loading...</div>
  if (error) return <div>{error}</div>
  data.text().then((actualData) => { // data.text is not a function on the browser
    console.log(actualData)

    return (
      <>
        {actualData}
      </>
    )
  })
})

Am I supposed to move the logic and .text() the fetch return in the api object?

@atellmer
Copy link
Owner

atellmer commented Jun 26, 2024

Am I supposed to move the logic and .text() the fetch return in the api object?

Yes, the API method must return an object with the final data if you want to display it in the render. Accordingly, all asynchronous data preparation logic should be inside the api method.

This is required in order to serialize data on the server and parse it on the client. And the Response object, when serialized, turns into {} and undefined appears when you access ({}).status.

@Coachonko
Copy link
Author

Ok. Thank you for the information. I'll do just that!

@atellmer atellmer added the enhancement New feature or request label Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants