Skip to content

Make sure query is refetched specified number of times#168

Merged
tannerlinsley merged 20 commits intoTanStack:masterfrom
cherniavskii:useQuery-retry-number
Feb 28, 2020
Merged

Make sure query is refetched specified number of times#168
tannerlinsley merged 20 commits intoTanStack:masterfrom
cherniavskii:useQuery-retry-number

Conversation

@cherniavskii
Copy link
Contributor

If retry is set to 1, I would expect useQuery to try once again after fail

@cherniavskii
Copy link
Contributor Author

That's weird, tests are passing on my machine :/

@tannerlinsley
Copy link
Member

Hmm Travis env issue maybe?

@cherniavskii
Copy link
Contributor Author

Can you check on your machine?

@tannerlinsley
Copy link
Member

Passes 😂

@cherniavskii
Copy link
Contributor Author

Hey Travis, WTF?

@tannerlinsley
Copy link
Member

Wait. I got it to fail locally. Try this:

const rendered = render(<Page />)

    await waitForElement(() => rendered.getByText('error'))

    rendered.debug()

    // query should fail `retry + 1` times, since first time isn't a "retry"
    await waitForElement(() => rendered.getByText('Failed 2 times'))

    rendered.debug()

@tannerlinsley
Copy link
Member

Looks like it never flushes the error state because the delay is so small? Thoughts?

@cherniavskii
Copy link
Contributor Author

To minimize the delay i've set retryDelay to 1.
Will check if it works

@tannerlinsley
Copy link
Member

Oooohhhh, I don't think the error status is reached until the very end. It stays in the loading state until the final failure or success.

@tannerlinsley
Copy link
Member

And I am pretty sure that is desired from the UI/UX standpoint out of the box.

@cherniavskii
Copy link
Contributor Author

Yeah, this is fine

@tannerlinsley
Copy link
Member

tannerlinsley commented Feb 27, 2020

Something still doesn't seem right tho. Try logging out the failureCount in the render function....

@tannerlinsley
Copy link
Member

tannerlinsley commented Feb 27, 2020

Oh wait. I forgot to build. I think it's good. This is what I have now:

it('should retry specified number of times', async () => {
    const queryFn = jest.fn()
    queryFn.mockImplementation(() => {
      return Promise.reject('Error test')
    })

    function Page() {
      const { status, failureCount } = useQuery('test', queryFn, {
        retry: 1,
        retryDelay: 1,
      })

      return (
        <div>
          <h1>{status}</h1>
          <h2>Failed {failureCount} times</h2>
        </div>
      )
    }

    const rendered = render(<Page />)

    rendered.getByText('loading')

    // query should fail `retry + 1` times, since first time isn't a "retry"
    await waitForElement(() => rendered.getByText('Failed 2 times'))

    rendered.debug()

    expect(queryFn).toHaveBeenCalledTimes(2)
  })

@cherniavskii
Copy link
Contributor Author

Let's try

@cherniavskii
Copy link
Contributor Author

Wow, I had to set 50s timeout to make it pass :D

I don't have a better solution for now, at least it works

@tannerlinsley
Copy link
Member

That just doesn't seem right though... like something else is wrong.

@tannerlinsley
Copy link
Member

Have you tested this in the browser and played with it there?

@cherniavskii
Copy link
Contributor Author

@cherniavskii
Copy link
Contributor Author

Same with tests - they're passing really fast locally.
That single test takes 4.587s

@tannerlinsley
Copy link
Member

That seems really strange. I want to get to the bottom of it.

@tannerlinsley
Copy link
Member

I think I found the reason. queryCache is shared among test instances, which are all running in parallel I think...

@tannerlinsley
Copy link
Member

Well, no, they're in isolated workers.... What is going on!!!

@iamchanii
Copy link

I'd experienced the issue what almost same it. the test passed on my machine but CI failed by timeout. so I had to google and found a solution.

if this project use Jest, would you like to try --maxWorkers=2 flag? it is working for my case without increase timeout seconds. but there is a slight drop in performance.

@cherniavskii
Copy link
Contributor Author

@iamchanii Unfortunately that didn't help - 9f46b36

@cherniavskii
Copy link
Contributor Author

queryCache is shared among test instances

I think this is true, looking into this...

@tannerlinsley
Copy link
Member

Alright, I found something that might help us debug. Change the queryKey to something other than test (or something unique to the entire test suite) and watch it pass 🤯

@cherniavskii
Copy link
Contributor Author

Yeah, I've noticed that too
But I found something else. Which is really weird. Would you have time for a call on Zoom?

@tannerlinsley
Copy link
Member

Yeah

@cherniavskii
Copy link
Contributor Author

I've sent you a link in Twitter DM

@tannerlinsley tannerlinsley merged commit 6eb3997 into TanStack:master Feb 28, 2020
@cherniavskii cherniavskii deleted the useQuery-retry-number branch February 28, 2020 17:38
@nx-cloud
Copy link

nx-cloud bot commented Apr 1, 2024

View your CI Pipeline Execution ↗ for commit 22ccd91.

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded <1s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-04-27 00:15:09 UTC

GEREGUR pushed a commit to GEREGUR/query that referenced this pull request Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments