fix(HMS-2261): show warning when polling reservation fails

[amirfefer]

When the reservation polling fails after a few attempts, (status != 200), the reservation was created already successfully, but there is no way to fetch the current launch status, this PR adds a warning , something like:

[mshriver]

/retest

[lzap]

LGTM

[ezr-ondrej]

Will this happen on first failure? If this would be on first failure, I would consider this confusing.

Also while trying to verify the above Q, I've hit https://tkdodo.eu/blog/breaking-react-querys-api-on-purpose which says the callbacks are not great way to do things and are deprecated in React Query v4. I guess you will make more sense out of whether we care or not, but they seem to think that using it for state management is a bad idea 🤷 :)

[ezr-ondrej]

Also the commit message is missing the delimeter ( : ) between fix and the actual subject and thus the commit message check is failing here :)

[amirfefer]

@ezr-ondrej,
We opened this issue because we break the API on purpose in the last tech discussion :) by hardcoding a different reservation id during polling, and getting a 404 error. this caused the UI progress to freeze.

Will this happen on first failure?

I'm not sure if I understand, could you explain?

Thanks for this blog post, we definitely need to start migrating our react-query 👍

Another solution here is using the timeout warning (but it is a bit odd to spam our backend with 404 / 500 until a timeout is reached)

[lzap]

Yeah you don’t have the context: @avitova figured out that if backend returns 404 (e.g. reservation is deleted somehow) the UI gets stuck because it tries to fetch /reservations/ID over and over again. It never stops. It is a valid bug, we should be checking HTTP codes thus this PR.

[ezr-ondrej]

Oh, I guess I understand now :)

The thing I'm worried about is if this sets an error on first failure (that react-query usually recovers from if that's just network issue, won't this completely stop the process, because now we set warning and thus stop polling?

[ezr-ondrej]

Another solution here is using the timeout warning (but it is a bit odd to spam our backend with 404 / 500 until a timeout is reached)

I agree this is kinda ok solution for now, until we will need to migrate to newer react-query, but I'm just worried we break other cases here. Not every error should stop the polling immediatly, right?

[lzap]

Not every error should stop the polling immediatly, right?

Examples?

[amirfefer]

@ezr-ondrej this change does not break the polling at the first attempt, there are 3 attempts by default, and only than the error raise.

We can increase the retries as we wish

https://tanstack.com/query/v4/docs/react/guides/query-retries

[amirfefer]

updated commit message.

[ezr-ondrej]

/retest

[ezr-ondrej]

In that case, perfect! thank you! 🙏