From 9b305a03e68cb0bdbd9af5b79753d06b4ff514f0 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin <alexander.polyankin@metabase.com> Date: Mon, 24 Jun 2024 12:07:58 -0400 Subject: [PATCH] Add a try-catch for MBQL lib diagnoseExpression (#44628) --- .../reproductions-3.cy.spec.js | 19 ++++++++++ .../v1/expressions/diagnostics.ts | 37 +++++++++++-------- 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/e2e/test/scenarios/question-reproductions/reproductions-3.cy.spec.js b/e2e/test/scenarios/question-reproductions/reproductions-3.cy.spec.js index cc44cb9e65a..e6df32013f5 100644 --- a/e2e/test/scenarios/question-reproductions/reproductions-3.cy.spec.js +++ b/e2e/test/scenarios/question-reproductions/reproductions-3.cy.spec.js @@ -32,6 +32,8 @@ import { appBar, openProductsTable, queryBuilderFooter, + enterCustomColumnDetails, + addCustomColumn, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID, PRODUCTS } = SAMPLE_DATABASE; @@ -475,6 +477,23 @@ describe("issue 40435", () => { }); }); +describe("issue 41381", () => { + beforeEach(() => { + restore(); + cy.signInAsNormalUser(); + }); + + it("should show an error message when adding a constant-only custom expression (metabase#41381)", () => { + openOrdersTable({ mode: "notebook" }); + addCustomColumn(); + enterCustomColumnDetails({ formula: "'Test'", name: "Constant" }); + popover().within(() => { + cy.findByText("Invalid expression").should("be.visible"); + cy.button("Done").should("be.disabled"); + }); + }); +}); + describe( "issue 42010 -- Unable to filter by mongo id", { tags: "@mongo" }, diff --git a/frontend/src/metabase-lib/v1/expressions/diagnostics.ts b/frontend/src/metabase-lib/v1/expressions/diagnostics.ts index 60257943aa8..5a8609137e4 100644 --- a/frontend/src/metabase-lib/v1/expressions/diagnostics.ts +++ b/frontend/src/metabase-lib/v1/expressions/diagnostics.ts @@ -144,22 +144,27 @@ export function diagnose({ const expressionMode: Lib.ExpressionMode = startRuleToExpressionModeMapping[startRule] ?? startRule; - const possibleError = Lib.diagnoseExpression( - query, - stageIndex, - expressionMode, - mbqlOrError, - expressionPosition, - ); - - if (possibleError) { - console.warn("diagnostic error", possibleError.message); - - // diagnoseExpression returns some messages which are user-friendly and some which are not. - // If the `friendly` flag is true, we can use the possibleError as-is; if not then use a generic message. - return possibleError.friendly - ? possibleError - : { message: t`Invalid expression` }; + try { + const possibleError = Lib.diagnoseExpression( + query, + stageIndex, + expressionMode, + mbqlOrError, + expressionPosition, + ); + + if (possibleError) { + console.warn("diagnostic error", possibleError.message); + + // diagnoseExpression returns some messages which are user-friendly and some which are not. + // If the `friendly` flag is true, we can use the possibleError as-is; if not then use a generic message. + return possibleError.friendly + ? possibleError + : { message: t`Invalid expression` }; + } + } catch (error) { + console.warn("diagnostic error", error); + return { message: t`Invalid expression` }; } return null; -- GitLab