fix: load table even if formula error is present #9299
Conversation
dosubot
bot
added
the
size:M
WalkthroughWalkthroughThe recent updates enhance error handling and introduce a mechanism for reloading table metadata across various components in the application. Key changes include the addition of a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant TablesStore
participant Api
User->>UI: Request data
UI->>TablesStore: Access data
TablesStore->>Api: Fetch metadata
Api-->>TablesStore: Return metadata
TablesStore-->>UI: Metadata ready
UI-->>User: Display data
User->>UI: Cause FORMULA_ERROR
UI->>TablesStore: Handle FORMULA_ERROR
TablesStore->>TablesStore: Reload table metadata
TablesStore->>Api: Fetch metadata
Api-->>TablesStore: Return metadata
TablesStore-->>UI: Updated metadata ready
UI-->>User: Display updated data
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/nc-gui/composables/useViewData.ts (2 hunks)
- packages/nc-gui/store/tables.ts (2 hunks)
- packages/nocodb-sdk/src/lib/globals.ts (1 hunks)
- packages/nocodb/src/db/formulav2/formulaQueryBuilderv2.ts (3 hunks)
- packages/nocodb/src/helpers/catchError.ts (3 hunks)
Additional comments not posted (7)
packages/nocodb-sdk/src/lib/globals.ts (1)
198-198: Addition ofFORMULA_ERRORtoNcErrorTypeis appropriate.The introduction of
FORMULA_ERRORenhances the granularity of error handling, allowing for more specific error management related to formulas.packages/nc-gui/store/tables.ts (1)
231-235: Addition ofreloadTableMetafunction is well-implemented.The function allows for dynamic reloading of table metadata, which enhances the functionality of the store.
packages/nc-gui/composables/useViewData.ts (1)
203-211: Enhanced error handling forFORMULA_ERRORis effective.The retry mechanism for
FORMULA_ERRORby reloading table metadata improves error recovery and aligns with the PR objectives.Ensure that the error handling logic is thoroughly tested to confirm its effectiveness.
Run the following script to verify the error handling logic:
packages/nocodb/src/helpers/catchError.ts (3)
433-434: Introduction ofExternalTimeoutclass is appropriate.This addition enhances the error hierarchy by accommodating timeout-specific errors, improving robustness.
584-587: Addition ofFORMULA_ERRORtoerrorHelpersis beneficial.This change allows for more informative feedback when formula errors occur, enhancing error reporting clarity.
817-822: Addition offormulaErrormethod inNcErrorclass is well-implemented.This method allows for structured handling of formula-specific errors, improving error management.
Ensure that the new error handling logic is thoroughly tested.
Run the following script to verify the new error handling logic:
Verification successful
Usage of
formulaErrormethod verifiedThe
formulaErrormethod is used in theformulaQueryBuilderv2.tsfile, indicating that it is integrated into the error handling logic for formula queries. Ensure that this integration is thoroughly tested to confirm its effectiveness.
- File:
packages/nocodb/src/db/formulav2/formulaQueryBuilderv2.ts
- Usage:
NcError.formulaError(e.message);Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the formulaError method. # Test: Search for formulaError usage. Expect: Correct implementation in error handling. rg --type js -A 5 $'NcError.formulaError'Length of output: 40
Script:
#!/bin/bash # Description: Verify the usage of the formulaError method in both JavaScript and TypeScript files. # Test: Search for formulaError usage in JavaScript and TypeScript files. rg --type-add 'ts:*.{ts,tsx}' --type ts --type js -A 5 $'formulaError'Length of output: 780
packages/nocodb/src/db/formulav2/formulaQueryBuilderv2.ts (1)
Line range hint
1377-1389: Improved error handling informulaQueryBuilderv2is effective.The differentiation of error types and use of
NcError.formulaErrorenhances error handling robustness.Ensure that the new error handling logic is thoroughly tested.
Run the following script to verify the new error handling logic:
|
Uffizzi Preview |
mertmit
force-pushed
the
nc-fix/formula-error-handling
branch
from
44f1991 to
08a12c7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
packages/nocodb/src/db/formulav2/formulaQueryBuilderv2.ts (1)
Line range hint
1377-1389: Ensure the error is thrown after logging.The enhanced error handling logic is good. However, the
NcError.formulaError(e.message)line does not throw the error, which might cause silent failures. Consider throwing the error after logging it.NcError.formulaError(e.message); + throw e;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- packages/nc-gui/composables/useViewData.ts (2 hunks)
- packages/nc-gui/store/tables.ts (2 hunks)
- packages/nocodb/src/db/formulav2/formulaQueryBuilderv2.ts (3 hunks)
- packages/nocodb/src/helpers/catchError.ts (2 hunks)
Additional comments not posted (5)
packages/nc-gui/composables/useViewData.ts (2)
19-20: LGTM!The refactoring improves code clarity and maintainability.
203-211: LGTM!The new error handling mechanism for
FORMULA_ERRORis correctly implemented.packages/nocodb/src/helpers/catchError.ts (2)
487-488: LGTM!The class is correctly implemented.
893-899: LGTM!The method is correctly implemented.
packages/nocodb/src/db/formulav2/formulaQueryBuilderv2.ts (1)
26-26: LGTM!The import statement for
ExternalTimeoutandNcErroris correct.The code changes are approved.
| const reloadTableMeta = async (tableId: string) => { | ||
| const { getMeta } = useMetas() | ||
|
|
||
| await getMeta(tableId, true) | ||
| } |
There was a problem hiding this comment.
Add error handling to reloadTableMeta.
The function is correctly implemented but lacks error handling. Consider adding a try-catch block to handle potential errors from getMeta.
const reloadTableMeta = async (tableId: string) => {
+ try {
const { getMeta } = useMetas()
await getMeta(tableId, true)
+ } catch (e) {
+ console.error(`Failed to reload table metadata for tableId: ${tableId}`, e)
+ }
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const reloadTableMeta = async (tableId: string) => { | |
| const { getMeta } = useMetas() | |
| await getMeta(tableId, true) | |
| } | |
| const reloadTableMeta = async (tableId: string) => { | |
| try { | |
| const { getMeta } = useMetas() | |
| await getMeta(tableId, true) | |
| } catch (e) { | |
| console.error(`Failed to reload table metadata for tableId: ${tableId}`, e) | |
| } | |
| } |
Change Summary
Provide summary of changes with issue number if any.
Change type
Test/ Verification
Provide summary of changes.
Additional information / screenshots (optional)
Anything for maintainers to be made aware of