-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(client-core): return null instead of undefined when query is cancelled by mutex #10274
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
base: master
Are you sure you want to change the base?
Conversation
…elled by mutex When multiple queries share the same mutexKey, newer queries invalidate older ones. Previously, cancelled queries would resolve to undefined because mutexPromise silently swallowed the MUTEX_ERROR without returning a value. This caused issues in useCubeQuery where resultSet would unexpectedly reset to undefined while isLoading remained false. Changes: - Add MutexCancelledError class for explicit mutex cancellation handling - Update mutexPromise to throw MutexCancelledError instead of swallowing errors - Catch MutexCancelledError in loadMethod and return null (a valid return type) Fixes #10261
|
Thanks for looking into this @igorlukanin - I didn't have a good understanding of the mutex behaviour. |
johnhunter
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solution makes sense to me. Just the type fallout to resolve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses an issue where queries cancelled by mutex operations were resolving to undefined, causing unexpected behavior in components like useCubeQuery. The fix introduces explicit error handling for mutex cancellations and returns null instead.
Key changes:
- Introduces
MutexCancelledErrorclass to explicitly represent mutex cancellation events - Updates
mutexPromisefunction to throwMutexCancelledErrorinstead of silently swallowing errors - Adds error handling in
loadMethodto catchMutexCancelledErrorand returnnull
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .then(requestInstance => mutexPromise(requestInstance.subscribe(loadImpl))) | ||
| .catch((error) => { | ||
| if (error instanceof MutexCancelledError) { | ||
| return null; |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The promise chain now returns null when MutexCancelledError is caught, but the public method signatures that call loadMethod (e.g., load, sql, meta, dryRun, cubeSql) don't include null in their return types. For example, the load method signature declares Promise<ResultSet<...>> but can now return Promise<ResultSet<...> | null>. This creates a type safety issue where consumers of the API may not handle the null case properly.
Consider updating all method signatures that use loadMethod to reflect that they can return null, or document this behavior explicitly.
| return null; | |
| // Propagate mutex cancellation as a rejected promise instead of resolving to null, | |
| // so that the promise's resolved type remains consistent with public method signatures. | |
| throw error; |
When multiple queries share the same mutexKey, newer queries invalidate older
ones. Previously, cancelled queries would resolve to undefined because
mutexPromise silently swallowed the MUTEX_ERROR without returning a value.
This caused issues in useCubeQuery where resultSet would unexpectedly reset
to undefined while isLoading remained false.
Changes:
Check List
Issue Reference this PR resolves
Fixes #10261
Conversation with Claude
claude.txt