From 5eb9c446c769e95bcc01594ba0534d6845b83255 Mon Sep 17 00:00:00 2001 From: David Russell Date: Mon, 16 Jun 2025 08:51:32 +0100 Subject: [PATCH 1/7] removes feature flag for gql errors and notifications --- .../ErrorsView/ErrorsView.test.tsx | 9 +------ .../CypherFrame/ErrorsView/ErrorsView.tsx | 26 +++++------------- .../Stream/CypherFrame/WarningsView.test.tsx | 9 +------ .../Stream/CypherFrame/WarningsView.tsx | 27 +++---------------- .../Stream/CypherFrame/gqlStatusUtils.ts | 2 +- .../Stream/CypherFrame/warningUtilts.ts | 12 +++++++++ .../modules/features/versionedFeatures.ts | 3 --- src/shared/modules/settings/settingsDuck.ts | 2 -- 8 files changed, 25 insertions(+), 65 deletions(-) diff --git a/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.test.tsx b/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.test.tsx index 7b319b6e3a2..1186e10bc80 100644 --- a/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.test.tsx +++ b/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.test.tsx @@ -38,8 +38,7 @@ const mount = (props: Partial, state?: any) => { params: {}, executeCmd: jest.fn(), setEditorContent: jest.fn(), - neo4jVersion: null, - gqlErrorsEnabled: true + neo4jVersion: null } const combinedProps = { @@ -118,9 +117,6 @@ describe('ErrorsView', () => { server: { version: '5.26.0' } - }, - settings: { - enableGqlErrorsAndNotifications: true } } @@ -161,9 +157,6 @@ describe('ErrorsView', () => { server: { version: '5.26.0' } - }, - settings: { - enableGqlErrorsAndNotifications: true } } diff --git a/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.tsx b/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.tsx index 465bba3b2c3..354d63346f8 100644 --- a/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.tsx +++ b/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.tsx @@ -57,14 +57,12 @@ import { import { BrowserError } from 'services/exceptions' import { deepEquals } from 'neo4j-arc/common' import { getSemanticVersion } from 'shared/modules/dbMeta/dbMetaDuck' -import { gte, SemVer } from 'semver' +import { SemVer } from 'semver' import { formatError, formatErrorGqlStatusObject, hasPopulatedGqlFields } from '../errorUtils' -import { FIRST_GQL_ERRORS_SUPPORT } from 'shared/modules/features/versionedFeatures' -import { shouldShowGqlErrorsAndNotifications } from 'shared/modules/settings/settingsDuck' export type ErrorsViewProps = { result: BrowserRequestResult @@ -74,7 +72,6 @@ export type ErrorsViewProps = { executeCmd: (cmd: string) => void setEditorContent: (cmd: string) => void depth?: number - gqlErrorsEnabled: boolean } class ErrorsViewComponent extends Component { @@ -92,8 +89,7 @@ class ErrorsViewComponent extends Component { executeCmd, setEditorContent, neo4jVersion, - depth = 0, - gqlErrorsEnabled + depth = 0 } = this.props const error = this.props.result as BrowserError @@ -101,10 +97,9 @@ class ErrorsViewComponent extends Component { return null } - const formattedError = - gqlErrorsEnabled && hasPopulatedGqlFields(error) - ? formatErrorGqlStatusObject(error) - : formatError(error) + const formattedError = hasPopulatedGqlFields(error) + ? formatErrorGqlStatusObject(error) + : formatError(error) if (!formattedError?.title) { return null @@ -176,18 +171,9 @@ class ErrorsViewComponent extends Component { } } -const gqlErrorsEnabled = (state: GlobalState): boolean => { - const featureEnabled = shouldShowGqlErrorsAndNotifications(state) - const version = getSemanticVersion(state) - return version - ? featureEnabled && gte(version, FIRST_GQL_ERRORS_SUPPORT) - : false -} - const mapStateToProps = (state: GlobalState) => ({ params: getParams(state), - neo4jVersion: getSemanticVersion(state), - gqlErrorsEnabled: gqlErrorsEnabled(state) + neo4jVersion: getSemanticVersion(state) }) const mapDispatchToProps = ( diff --git a/src/browser/modules/Stream/CypherFrame/WarningsView.test.tsx b/src/browser/modules/Stream/CypherFrame/WarningsView.test.tsx index c05ca983e77..85d6601596e 100644 --- a/src/browser/modules/Stream/CypherFrame/WarningsView.test.tsx +++ b/src/browser/modules/Stream/CypherFrame/WarningsView.test.tsx @@ -36,8 +36,7 @@ const withProvider = (store: any, children: any) => { const mount = (props: DeepPartial, state?: any) => { const defaultProps: WarningsViewProps = { result: null, - bus: createBus(), - gqlWarningsEnabled: false + bus: createBus() } const combinedProps = { @@ -141,9 +140,6 @@ describe('WarningsView', () => { server: { version: '5.23.0' } - }, - settings: { - enableGqlErrorsAndNotifications: true } } @@ -237,9 +233,6 @@ describe('WarningsView', () => { server: { version: '5.23.0' } - }, - settings: { - enableGqlErrorsAndNotifications: true } } diff --git a/src/browser/modules/Stream/CypherFrame/WarningsView.tsx b/src/browser/modules/Stream/CypherFrame/WarningsView.tsx index 5c554e44272..3f3d9d0955f 100644 --- a/src/browser/modules/Stream/CypherFrame/WarningsView.tsx +++ b/src/browser/modules/Stream/CypherFrame/WarningsView.tsx @@ -37,17 +37,13 @@ import { deepEquals } from 'neo4j-arc/common' import { formatSummaryFromGqlStatusObjects, formatSummaryFromNotifications, - FormattedNotification + FormattedNotification, + hasPopulatedGqlFields } from './warningUtilts' import { NotificationSeverityLevel, QueryResult } from 'neo4j-driver-core' import { connect } from 'react-redux' import { withBus } from 'react-suber' -import { GlobalState } from 'shared/globalState' import { Bus } from 'suber' -import { getSemanticVersion } from 'shared/modules/dbMeta/dbMetaDuck' -import { gte } from 'semver' -import { FIRST_GQL_NOTIFICATIONS_SUPPORT } from 'shared/modules/features/versionedFeatures' -import { shouldShowGqlErrorsAndNotifications } from 'shared/modules/settings/settingsDuck' const getWarningComponent = (severity?: string | NotificationSeverityLevel) => { if (severity === 'ERROR') { @@ -64,7 +60,6 @@ const getWarningComponent = (severity?: string | NotificationSeverityLevel) => { export type WarningsViewProps = { result?: QueryResult | null bus: Bus - gqlWarningsEnabled: boolean } class WarningsViewComponent extends Component { @@ -82,7 +77,7 @@ class WarningsViewComponent extends Component { return null const { summary } = this.props.result - const notifications = this.props.gqlWarningsEnabled + const notifications = hasPopulatedGqlFields(summary) ? formatSummaryFromGqlStatusObjects(summary) : formatSummaryFromNotifications(summary) const { text: cypher = '' } = summary.query @@ -130,21 +125,7 @@ class WarningsViewComponent extends Component { } } -const gqlWarningsEnabled = (state: GlobalState): boolean => { - const featureEnabled = shouldShowGqlErrorsAndNotifications(state) - const version = getSemanticVersion(state) - return version - ? featureEnabled && gte(version, FIRST_GQL_NOTIFICATIONS_SUPPORT) - : false -} - -const mapStateToProps = (state: GlobalState) => ({ - gqlWarningsEnabled: gqlWarningsEnabled(state) -}) - -export const WarningsView = withBus( - connect(mapStateToProps, null)(WarningsViewComponent) -) +export const WarningsView = withBus(connect(null, null)(WarningsViewComponent)) export class WarningsStatusbar extends Component { shouldComponentUpdate() { diff --git a/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.ts b/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.ts index 72095e80554..5ade00031e0 100644 --- a/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.ts +++ b/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.ts @@ -31,7 +31,7 @@ const formatPropertyFromStatusDescripton = ( ): string | undefined => { const matches = gqlStatusDescription?.match( - /^(?:error|info|warn):\s(.+?)(?:\.(.+?))?\.?$/ + /^(?:(?:error|info|warn):\s)?(.+?)(?:\.(.+?))?\.?$/s ) ?? [] return matches[index] === undefined diff --git a/src/browser/modules/Stream/CypherFrame/warningUtilts.ts b/src/browser/modules/Stream/CypherFrame/warningUtilts.ts index 6b373180910..df7872ee223 100644 --- a/src/browser/modules/Stream/CypherFrame/warningUtilts.ts +++ b/src/browser/modules/Stream/CypherFrame/warningUtilts.ts @@ -75,6 +75,18 @@ const mapNotificationsToFormattedNotifications = ( const SEVERITY_LEVELS = ['ERROR', 'WARNING', 'INFORMATION'] +export const hasPopulatedGqlFields = ( + resultSummary: Partial +): resultSummary is Partial & { + gqlStatusObjects: GqlStatusObject[] + notifications: Notification[] +} => { + return ( + 'gqlStatusObjects' in resultSummary && + resultSummary.gqlStatusObjects !== undefined + ) +} + export const formatSummaryFromNotifications = ( resultSummary?: Partial ): FormattedNotification[] => { diff --git a/src/shared/modules/features/versionedFeatures.ts b/src/shared/modules/features/versionedFeatures.ts index fa61293b5f7..187e9b121dd 100644 --- a/src/shared/modules/features/versionedFeatures.ts +++ b/src/shared/modules/features/versionedFeatures.ts @@ -32,9 +32,6 @@ export const FIRST_MULTI_DB_SUPPORT = NEO4J_4_0 // compatible bolt server. export const FIRST_NO_MULTI_DB_SUPPORT = '3.4.0' -export const FIRST_GQL_NOTIFICATIONS_SUPPORT = '5.23.0' -export const FIRST_GQL_ERRORS_SUPPORT = '5.26.0' - export const getShowCurrentUserProcedure = (serverVersion: string) => { const serverVersionGuessed = guessSemverVersion(serverVersion) diff --git a/src/shared/modules/settings/settingsDuck.ts b/src/shared/modules/settings/settingsDuck.ts index d9fd71e2641..2b2eaaa4fc3 100644 --- a/src/shared/modules/settings/settingsDuck.ts +++ b/src/shared/modules/settings/settingsDuck.ts @@ -94,8 +94,6 @@ export const getAllowUserStats = (state: GlobalState): boolean => state[NAME].allowUserStats ?? initialState.allowUserStats export const shouldShowWheelZoomInfo = (state: GlobalState) => state[NAME].showWheelZoomInfo -export const shouldShowGqlErrorsAndNotifications = (state: any) => - state[NAME].enableGqlErrorsAndNotifications // Ideally the string | number types would be only numbers // but they're saved as strings in the settings component From 773725eda46fda3ea0f7bd236e55ce9bbd3fcd13 Mon Sep 17 00:00:00 2001 From: David Russell Date: Mon, 16 Jun 2025 10:15:11 +0100 Subject: [PATCH 2/7] refines rege without dotall flag --- src/browser/modules/Stream/CypherFrame/gqlStatusUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.ts b/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.ts index 5ade00031e0..d0026ec69c4 100644 --- a/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.ts +++ b/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.ts @@ -31,7 +31,7 @@ const formatPropertyFromStatusDescripton = ( ): string | undefined => { const matches = gqlStatusDescription?.match( - /^(?:(?:error|info|warn):\s)?(.+?)(?:\.(.+?))?\.?$/s + /^(?:(?:error|info|warn):\s)?([\s\S]+?)(?:\.([\s\S]+?))?\.?$/ ) ?? [] return matches[index] === undefined From 0e1211d3bf76d7718f5c478878bf5ac83647feec Mon Sep 17 00:00:00 2001 From: David Russell Date: Mon, 16 Jun 2025 10:42:48 +0100 Subject: [PATCH 3/7] updates unit tests --- .../Stream/CypherFrame/gqlStatusUtils.test.ts | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.test.ts b/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.test.ts index c1f14a0222b..9824ce0eef6 100644 --- a/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.test.ts +++ b/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.test.ts @@ -58,19 +58,37 @@ describe('gql status formatting', () => { ) }) - test('formats a title from a gql status description with no matches correctly', () => { + test('formats a description with line breaks correctly', () => { + const gqlStatusDescription = `error: general processing exception - unexpected error. The shortest path algorithm does not work when the start and end nodes are the same. This can happen if you +perform a shortestPath search after a cartesian product that might have the same start and end nodes for some +of the rows passed to shortestPath.` + + const result = + formatDescriptionFromGqlStatusDescription(gqlStatusDescription) + + expect(result) + .toEqual(`The shortest path algorithm does not work when the start and end nodes are the same. This can happen if you +perform a shortestPath search after a cartesian product that might have the same start and end nodes for some +of the rows passed to shortestPath.`) + }) + + test('formats a title from a gql status description with no error|info|warn prefix correctly', () => { const gqlStatusDescription = - 'Unfortunately, no one can be told what the Matrix is. You have to see it for yourself' + 'Unfortunately, no one can be told what the Matrix is. You have to see it for yourself.' + const result = formatTitleFromGqlStatusDescription(gqlStatusDescription) - expect(result).toEqual('') + expect(result).toEqual( + 'Unfortunately, no one can be told what the Matrix is' + ) }) - test('formats a description from a gql status description with no matches correctly', () => { - const gqlStatusDescription = 'Believe the unbelievable' - const result = - formatDescriptionFromGqlStatusDescription(gqlStatusDescription) + test('formats a description from a gql status description with no error|info|warn correctly', () => { + const gqlStatusDescription = + 'Unfortunately, no one can be told what the Matrix is. You have to see it for yourself.' + + const result = formatTitleFromGqlStatusDescription(gqlStatusDescription) - expect(result).toEqual('') + expect(result).toEqual('You have to see it for yourself.') }) }) From 5db7512060f46b300df99fabd7c1c230cbb29ef6 Mon Sep 17 00:00:00 2001 From: David Russell Date: Mon, 16 Jun 2025 11:03:40 +0100 Subject: [PATCH 4/7] fixes gql title formatting --- src/browser/modules/Stream/CypherFrame/errorUtils.ts | 5 ++++- .../modules/Stream/CypherFrame/gqlStatusUtils.test.ts | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/browser/modules/Stream/CypherFrame/errorUtils.ts b/src/browser/modules/Stream/CypherFrame/errorUtils.ts index 8fd6b757157..bad62b1127e 100644 --- a/src/browser/modules/Stream/CypherFrame/errorUtils.ts +++ b/src/browser/modules/Stream/CypherFrame/errorUtils.ts @@ -59,7 +59,10 @@ const mapBrowserErrorToFormattedError = ( ) const title = isNonEmptyString(gqlStatusTitle) ? gqlStatusTitle : description return { - title: isNonEmptyString(title) ? `${gqlStatus}: ${title}` : gqlStatus, + title: + isNonEmptyString(title) && title !== gqlStatus + ? `${gqlStatus}: ${title}` + : gqlStatus, description } } diff --git a/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.test.ts b/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.test.ts index 9824ce0eef6..3b260e59786 100644 --- a/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.test.ts +++ b/src/browser/modules/Stream/CypherFrame/gqlStatusUtils.test.ts @@ -87,7 +87,8 @@ of the rows passed to shortestPath.`) const gqlStatusDescription = 'Unfortunately, no one can be told what the Matrix is. You have to see it for yourself.' - const result = formatTitleFromGqlStatusDescription(gqlStatusDescription) + const result = + formatDescriptionFromGqlStatusDescription(gqlStatusDescription) expect(result).toEqual('You have to see it for yourself.') }) From 0f7f53dbe1f63df38a0da6504f135c1a6c7a3588 Mon Sep 17 00:00:00 2001 From: David Russell Date: Mon, 16 Jun 2025 15:12:23 +0100 Subject: [PATCH 5/7] fixes text selector in e2e test --- e2e_tests/integration/multistatements.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/e2e_tests/integration/multistatements.spec.ts b/e2e_tests/integration/multistatements.spec.ts index d9e9386ab91..94ba1663f83 100644 --- a/e2e_tests/integration/multistatements.spec.ts +++ b/e2e_tests/integration/multistatements.spec.ts @@ -32,6 +32,7 @@ describe('Multi statements', () => { after(() => { cy.disableMultiStatement() }) + it('can connect', () => { const password = Cypress.config('password') cy.connect('neo4j', password) @@ -69,7 +70,7 @@ describe('Multi statements', () => { ) cy.get('[data-testid="frameContents"]', { timeout: 10000 }) .first() - .should('contain', 'Error') + .should('contain', 'ERROR') cy.get('[data-testid="navigationSettings"]').click() cy.get('[data-testid="setting-enableMultiStatementMode"]').click() @@ -94,6 +95,7 @@ describe('Multi statements', () => { .first() .should('contain', 'ERROR') }) + it('Takes any statements (not just valid cypher and client commands)', () => { cy.executeCommand(':clear') const query = 'RETURN 1; hello1; RETURN 2; hello2;' @@ -112,6 +114,7 @@ describe('Multi statements', () => { .first() .should('contain', 'ERROR') }) + if (Cypress.config('serverVersion') >= 4.1) { if (isEnterpriseEdition()) { it('Can use :use command in multi-statements', () => { From bb270fe9bf6bd98e8ab08fbd4d60aa1047cc55d9 Mon Sep 17 00:00:00 2001 From: David Russell Date: Mon, 16 Jun 2025 16:37:13 +0100 Subject: [PATCH 6/7] updates e2e test selectors --- e2e_tests/integration/editor.spec.ts | 2 +- e2e_tests/integration/params.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/e2e_tests/integration/editor.spec.ts b/e2e_tests/integration/editor.spec.ts index dfbe5eaa41c..02b7bb7a85c 100644 --- a/e2e_tests/integration/editor.spec.ts +++ b/e2e_tests/integration/editor.spec.ts @@ -42,7 +42,7 @@ describe('Cypher Editor', () => { // It can take a little while for the label meta-data to update in the background cy.getEditor().type(selectAllAndDelete) cy.executeCommand('return extraTimeForMetadataupdate') - cy.resultContains('extraTimeForMetadataupdate') + cy.resultContains('ERROR') cy.wait(5000) cy.getEditor().type(selectAllAndDelete) diff --git a/e2e_tests/integration/params.spec.ts b/e2e_tests/integration/params.spec.ts index 05fa97f205c..8d16b96914c 100644 --- a/e2e_tests/integration/params.spec.ts +++ b/e2e_tests/integration/params.spec.ts @@ -150,8 +150,8 @@ testData.forEach(testData => { it('can generate a set params template to use if query is missing params', () => { cy.executeCommand(':clear') cy.executeCommand('return $test1, $test2') - const expectedMessage = `Expected parameter(s): test1, test2` - cy.get('[data-testid="cypherFrameErrorMessage"]', { timeout: 20000 }) + const expectedMessage = `Use this template to add missing parameter(s):` + cy.get('[data-testid="frameContents"]', { timeout: 20000 }) .first() .should('contain', expectedMessage) From e9ada2a6139b685165b220c739906827cf1c1c9f Mon Sep 17 00:00:00 2001 From: David Russell Date: Thu, 3 Jul 2025 16:26:29 +0100 Subject: [PATCH 7/7] updates errors and notifications to show only on 2025 onwards --- .../ErrorsView/ErrorsView.test.tsx | 8 +- .../CypherFrame/ErrorsView/ErrorsView.tsx | 130 ++++++++--- .../__snapshots__/ErrorsView.test.tsx.snap | 211 ++++++++---------- .../Stream/CypherFrame/WarningsView.test.tsx | 4 +- .../Stream/CypherFrame/WarningsView.tsx | 26 ++- .../__snapshots__/WarningsView.test.tsx.snap | 8 +- .../Stream/CypherFrame/errorUtils.test.ts | 56 ++++- .../modules/Stream/CypherFrame/errorUtils.ts | 52 ++++- .../Stream/CypherFrame/warningUtils.test.ts | 11 +- .../Stream/CypherFrame/warningUtilts.ts | 23 +- src/browser/modules/Stream/styled.tsx | 6 +- .../modules/features/versionedFeatures.ts | 2 + src/shared/services/gqlUtils.ts | 11 + 13 files changed, 355 insertions(+), 193 deletions(-) create mode 100644 src/shared/services/gqlUtils.ts diff --git a/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.test.tsx b/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.test.tsx index 1186e10bc80..4c75281b174 100644 --- a/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.test.tsx +++ b/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.test.tsx @@ -96,7 +96,7 @@ describe('ErrorsView', () => { expect(container).toMatchSnapshot() }) - test('does display an error for gql status codes', () => { + test('does display an error for GQL status codes', () => { // Given const error: BrowserError = { code: 'Test.Error', @@ -115,7 +115,7 @@ describe('ErrorsView', () => { const state = { meta: { server: { - version: '5.26.0' + version: '5.27.0' } } } @@ -127,7 +127,7 @@ describe('ErrorsView', () => { expect(container).toMatchSnapshot() }) - test('does display a nested error for gql status codes', () => { + test('does display a nested error for GQL status codes', () => { // Given const error: BrowserError = { code: 'Test.Error', @@ -155,7 +155,7 @@ describe('ErrorsView', () => { const state = { meta: { server: { - version: '5.26.0' + version: '5.27.0' } } } diff --git a/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.tsx b/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.tsx index 354d63346f8..e69f6993048 100644 --- a/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.tsx +++ b/src/browser/modules/Stream/CypherFrame/ErrorsView/ErrorsView.tsx @@ -59,10 +59,48 @@ import { deepEquals } from 'neo4j-arc/common' import { getSemanticVersion } from 'shared/modules/dbMeta/dbMetaDuck' import { SemVer } from 'semver' import { + flattenAndInvertErrors, formatError, - formatErrorGqlStatusObject, - hasPopulatedGqlFields + FormattedError } from '../errorUtils' +import { gqlErrorsAndNotificationsEnabled } from 'services/gqlUtils' +import styled from 'styled-components' + +const StyledErrorsViewInnerComponentContent = styled.div<{ nested: boolean }>` + padding-left: ${props => (props.nested ? '20px' : '0')}; +` + +type ErrorsViewInnerProps = { + formattedError: FormattedError + nested?: boolean +} + +class ErrorsViewInnerComponent extends Component { + render(): null | JSX.Element { + const { formattedError, nested = false } = this.props + + return ( + + + {!nested && ( + + ERROR + {formattedError.title} + + )} + {nested &&
{formattedError.title}
} + + {formattedError.description && ( + + + {formattedError?.description} + + + )} + + ) + } +} export type ErrorsViewProps = { result: BrowserRequestResult @@ -72,13 +110,26 @@ export type ErrorsViewProps = { executeCmd: (cmd: string) => void setEditorContent: (cmd: string) => void depth?: number + gqlErrorsAndNotificationsEnabled?: boolean } -class ErrorsViewComponent extends Component { - shouldComponentUpdate(props: ErrorsViewProps): boolean { +type ErrorsViewState = { + nestedErrorsToggled: boolean +} + +class ErrorsViewComponent extends Component { + state = { + nestedErrorsToggled: false + } + + shouldComponentUpdate( + props: ErrorsViewProps, + state: ErrorsViewState + ): boolean { return ( !deepEquals(props.result, this.props.result) || - !deepEquals(props.params, this.props.params) + !deepEquals(props.params, this.props.params) || + !deepEquals(state, this.state) ) } @@ -89,17 +140,32 @@ class ErrorsViewComponent extends Component { executeCmd, setEditorContent, neo4jVersion, - depth = 0 + gqlErrorsAndNotificationsEnabled = false } = this.props const error = this.props.result as BrowserError + + const invertedErrors = flattenAndInvertErrors( + error, + gqlErrorsAndNotificationsEnabled + ) + const [deepestError] = invertedErrors + const nestedErrors = invertedErrors.slice(1) + const togglable = nestedErrors.length > 0 + const setNestedErrorsToggled = (toggled: boolean) => { + this.setState({ + nestedErrorsToggled: toggled + }) + } + if (!error) { return null } - const formattedError = hasPopulatedGqlFields(error) - ? formatErrorGqlStatusObject(error) - : formatError(error) + const formattedError = formatError( + deepestError, + gqlErrorsAndNotificationsEnabled + ) if (!formattedError?.title) { return null @@ -110,24 +176,9 @@ class ErrorsViewComponent extends Component { } return ( - 0}> + - - {depth === 0 && ( - ERROR - )} - {formattedError.title} - - {formattedError.description && ( - - - {formattedError?.description} - - - )} - {formattedError.innerError && ( - - )} + {isUnknownProcedureError(error) && ( { } /> )} + {togglable && ( + + + setNestedErrorsToggled(!this.state.nestedErrorsToggled) + } + > +   + {this.state.nestedErrorsToggled ? 'Show less' : 'Show more'} + + + )} + {this.state.nestedErrorsToggled && + nestedErrors.map((nestedError, index) => ( + + ))} ) @@ -173,7 +247,8 @@ class ErrorsViewComponent extends Component { const mapStateToProps = (state: GlobalState) => ({ params: getParams(state), - neo4jVersion: getSemanticVersion(state) + neo4jVersion: getSemanticVersion(state), + gqlErrorsAndNotificationsEnabled: gqlErrorsAndNotificationsEnabled(state) }) const mapDispatchToProps = ( @@ -192,6 +267,7 @@ const mapDispatchToProps = ( } } } + export const ErrorsView = withBus( connect(mapStateToProps, mapDispatchToProps)(ErrorsViewComponent) ) diff --git a/src/browser/modules/Stream/CypherFrame/ErrorsView/__snapshots__/ErrorsView.test.tsx.snap b/src/browser/modules/Stream/CypherFrame/ErrorsView/__snapshots__/ErrorsView.test.tsx.snap index 3cd5929ddd4..7e4def5d96d 100644 --- a/src/browser/modules/Stream/CypherFrame/ErrorsView/__snapshots__/ErrorsView.test.tsx.snap +++ b/src/browser/modules/Stream/CypherFrame/ErrorsView/__snapshots__/ErrorsView.test.tsx.snap @@ -5,34 +5,38 @@ exports[`ErrorsView displays nothing if no errors 1`] = `
`; exports[`ErrorsView displays procedure link if unknown procedure 1`] = `
- ERROR +
+ ERROR +
+

+ Neo.ClientError.Procedure.ProcedureNotFound +

-

- Neo.ClientError.Procedure.ProcedureNotFound -

-
-
- 
-          not found
-
+ 
+            not found
+
+
`; -exports[`ErrorsView does display a nested error for gql status codes 1`] = ` +exports[`ErrorsView does display a nested error for GQL status codes 1`] = `
- ERROR +
+ ERROR +
+

+ 22N27: Data exception - invalid entity type +

-

- 42N51: Syntax error or access rule violation - invalid parameter -

-
-
- 
-          Invalid parameter $\`param\`.
-
+ 
+            Invalid input '******' for $\`param\`. Expected to be STRING.
+
+
-
-
-

- 22G03: Data exception - invalid value type -

-
- -
-
-
-

- 22N27: Data exception - invalid entity type -

-
-
- 
-                  Invalid input '******' for $\`param\`. Expected to be STRING.
-
-
-
-
-
+   + Show more +
@@ -137,71 +110,79 @@ exports[`ErrorsView does display a nested error for gql status codes 1`] = ` exports[`ErrorsView does display an error 1`] = `
- ERROR +
+ ERROR +
+

+ Test.Error +

-

- Test.Error -

-
-
- 
-          Test error description
-
+ 
+            Test error description
+
+
`; -exports[`ErrorsView does display an error for gql status codes 1`] = ` +exports[`ErrorsView does display an error for GQL status codes 1`] = `
- ERROR +
+ ERROR +
+

+ 22N14: Data exception - invalid temporal value combination +

-

- 22N14: Data exception - invalid temporal value combination -

-
-
- 
-          Cannot select both epochSeconds and 'datetime'.
-
+ 
+            Cannot select both epochSeconds and 'datetime'.
+
+
diff --git a/src/browser/modules/Stream/CypherFrame/WarningsView.test.tsx b/src/browser/modules/Stream/CypherFrame/WarningsView.test.tsx index 85d6601596e..98743b7b049 100644 --- a/src/browser/modules/Stream/CypherFrame/WarningsView.test.tsx +++ b/src/browser/modules/Stream/CypherFrame/WarningsView.test.tsx @@ -138,7 +138,7 @@ describe('WarningsView', () => { const state = { meta: { server: { - version: '5.23.0' + version: '5.27.0' } } } @@ -231,7 +231,7 @@ describe('WarningsView', () => { const state = { meta: { server: { - version: '5.23.0' + version: '5.27.0' } } } diff --git a/src/browser/modules/Stream/CypherFrame/WarningsView.tsx b/src/browser/modules/Stream/CypherFrame/WarningsView.tsx index 3f3d9d0955f..484db187b31 100644 --- a/src/browser/modules/Stream/CypherFrame/WarningsView.tsx +++ b/src/browser/modules/Stream/CypherFrame/WarningsView.tsx @@ -34,16 +34,13 @@ import { StyledCypherInfoMessage } from '../styled' import { deepEquals } from 'neo4j-arc/common' -import { - formatSummaryFromGqlStatusObjects, - formatSummaryFromNotifications, - FormattedNotification, - hasPopulatedGqlFields -} from './warningUtilts' +import { formatSummary, FormattedNotification } from './warningUtilts' import { NotificationSeverityLevel, QueryResult } from 'neo4j-driver-core' import { connect } from 'react-redux' import { withBus } from 'react-suber' import { Bus } from 'suber' +import { gqlErrorsAndNotificationsEnabled } from 'services/gqlUtils' +import { GlobalState } from 'shared/globalState' const getWarningComponent = (severity?: string | NotificationSeverityLevel) => { if (severity === 'ERROR') { @@ -60,6 +57,7 @@ const getWarningComponent = (severity?: string | NotificationSeverityLevel) => { export type WarningsViewProps = { result?: QueryResult | null bus: Bus + gqlErrorsAndNotificationsEnabled?: boolean } class WarningsViewComponent extends Component { @@ -76,10 +74,12 @@ class WarningsViewComponent extends Component { ) return null + const { gqlErrorsAndNotificationsEnabled = false } = this.props const { summary } = this.props.result - const notifications = hasPopulatedGqlFields(summary) - ? formatSummaryFromGqlStatusObjects(summary) - : formatSummaryFromNotifications(summary) + const notifications = formatSummary( + summary, + gqlErrorsAndNotificationsEnabled + ) const { text: cypher = '' } = summary.query if (!notifications || !cypher) { @@ -125,7 +125,13 @@ class WarningsViewComponent extends Component { } } -export const WarningsView = withBus(connect(null, null)(WarningsViewComponent)) +const mapStateToProps = (state: GlobalState) => ({ + gqlErrorsAndNotificationsEnabled: gqlErrorsAndNotificationsEnabled(state) +}) + +export const WarningsView = withBus( + connect(mapStateToProps, null)(WarningsViewComponent) +) export class WarningsStatusbar extends Component { shouldComponentUpdate() { diff --git a/src/browser/modules/Stream/CypherFrame/__snapshots__/WarningsView.test.tsx.snap b/src/browser/modules/Stream/CypherFrame/__snapshots__/WarningsView.test.tsx.snap index 1f4580d240f..2ad7bfac2d5 100644 --- a/src/browser/modules/Stream/CypherFrame/__snapshots__/WarningsView.test.tsx.snap +++ b/src/browser/modules/Stream/CypherFrame/__snapshots__/WarningsView.test.tsx.snap @@ -5,7 +5,7 @@ exports[`WarningsView displays nothing if no notifications 1`] = `
`; exports[`WarningsView does display a warning 1`] = `
{ + test('formats a legacy error with no gql fields correctly', () => { + const error = { + type: 'Neo4jError' as ErrorType, + message: 'epochSeconds cannot be selected together with datetime.', + code: 'Neo.ClientError.Statement.ArgumentError' + } + + const result = formatError(error, false) + expect(result).toEqual({ + description: 'epochSeconds cannot be selected together with datetime.', + title: 'Neo.ClientError.Statement.ArgumentError' + }) + }) + test('formats an error with no gql fields correctly', () => { const error = { type: 'Neo4jError' as ErrorType, @@ -29,7 +43,7 @@ describe('error formatting', () => { code: 'Neo.ClientError.Statement.ArgumentError' } - const result = formatError(error) + const result = formatError(error, true) expect(result).toEqual({ description: 'epochSeconds cannot be selected together with datetime.', title: 'Neo.ClientError.Statement.ArgumentError' @@ -44,7 +58,7 @@ describe('error formatting', () => { code: 'Neo.DatabaseError.Statement.ExecutionFailed' } - const result = formatError(error) + const result = formatError(error, true) expect(result).toEqual({ description: 'The shortest path algorithm does not work when the start and end nodes are the same. This can happen if you perform a shortestPath search after a cartesian product that might have the same start and end nodes for some of the rows passed to shortestPath. If you would rather not experience this exception, and can accept the possibility of missing results for those rows, disable this in the Neo4j configuration by setting `dbms.cypher.forbid_shortestpath_common_nodes` to false. If you cannot accept missing results, and really want the shortestPath between two common nodes, then re-write the query using a standard Cypher variable length pattern expression followed by ordering by path length and limiting to one result.', @@ -52,6 +66,32 @@ describe('error formatting', () => { }) }) + test('formats a legacy error with gql fields correctly', () => { + const error = { + type: 'Neo4jError' as ErrorType, + message: 'Expected parameter(s): param', + code: 'Neo.ClientError.Statement.ParameterMissing', + gqlStatus: '42N51', + gqlStatusDescription: + 'error: syntax error or access rule violation - invalid parameter. Invalid parameter $`param`.', + cause: { + gqlStatus: '22G03', + gqlStatusDescription: '22G03', + cause: { + gqlStatus: '22N27', + gqlStatusDescription: + "error: data exception - invalid entity type. Invalid input '******' for $`param`. Expected to be STRING." + } + } + } + + const result = formatError(error, false) + expect(result).toEqual({ + description: 'Expected parameter(s): param', + title: 'Neo.ClientError.Statement.ParameterMissing' + }) + }) + test('formats a gql error correctly', () => { const error = { type: 'Neo4jError' as ErrorType, @@ -71,7 +111,7 @@ describe('error formatting', () => { } } - const result = formatErrorGqlStatusObject(error) + const result = formatError(error, true) expect(result).toEqual({ description: 'Invalid parameter $`param`.', innerError: { @@ -102,7 +142,7 @@ describe('error formatting', () => { } } - const result = formatErrorGqlStatusObject(error) + const result = formatError(error, true) expect(result).toEqual({ description: '', title: '22007: Data exception - invalid date, time, or datetime format', @@ -124,7 +164,7 @@ describe('error formatting', () => { cause: undefined } - const result = formatErrorGqlStatusObject(error) + const result = formatError(error, true) expect(result).toEqual({ description: '', title: '22G03', @@ -143,7 +183,7 @@ describe('error formatting', () => { cause: undefined } - const result = formatErrorGqlStatusObject(error) + const result = formatError(error, true) expect(result).toEqual({ description: "Invalid input '******' for $`param`. Expected to be STRING.", @@ -164,7 +204,7 @@ describe('error formatting', () => { cause: undefined } - const result = formatErrorGqlStatusObject(error) + const result = formatError(error, true) expect(result).toEqual({ description: "Cannot find the shortest path when the start and end nodes are the same. To enable this behavior, set 'dbms.cypher.forbid_shortestpath_common_nodes' to false.", diff --git a/src/browser/modules/Stream/CypherFrame/errorUtils.ts b/src/browser/modules/Stream/CypherFrame/errorUtils.ts index bad62b1127e..9110ab01f7c 100644 --- a/src/browser/modules/Stream/CypherFrame/errorUtils.ts +++ b/src/browser/modules/Stream/CypherFrame/errorUtils.ts @@ -24,6 +24,7 @@ import { formatTitleFromGqlStatusDescription } from './gqlStatusUtils' import { BrowserError } from 'services/exceptions' +import { cloneDeep } from 'lodash-es' export function isBrowserError(object: unknown): object is BrowserError { if (object !== null && typeof object === 'object') { @@ -38,7 +39,7 @@ export function isBrowserError(object: unknown): object is BrowserError { return false } -type FormattedError = { +export type FormattedError = { title?: string description?: string innerError?: Pick< @@ -67,7 +68,7 @@ const mapBrowserErrorToFormattedError = ( } } -export const hasPopulatedGqlFields = ( +const hasPopulatedGqlFields = ( error: BrowserError | Error ): error is BrowserError & { gqlStatus: string @@ -83,16 +84,14 @@ export const hasPopulatedGqlFields = ( ) } -export const formatErrorGqlStatusObject = ( - error: BrowserError -): FormattedError => { +const formatErrorGqlStatusObject = (error: BrowserError): FormattedError => { return { ...mapBrowserErrorToFormattedError(error), innerError: error.cause } } -export const formatError = (error: BrowserError): FormattedError => { +const formatLegacyError = (error: BrowserError): FormattedError => { const { code: title, message: description } = error return { @@ -100,3 +99,44 @@ export const formatError = (error: BrowserError): FormattedError => { description } } + +export const formatError = ( + error: BrowserError, + gqlErrorsAndNotificationsEnabled: boolean +): FormattedError => { + if (hasPopulatedGqlFields(error) && gqlErrorsAndNotificationsEnabled) { + return formatErrorGqlStatusObject(error) + } + + return formatLegacyError(error) +} + +export const flattenAndInvertErrors = ( + error: BrowserError, + gqlErrorsAndNotificationsEnabled: boolean +): BrowserError[] => { + const flattenErrorsToArray = ( + currentError: BrowserError, + errors: BrowserError[] = [] + ): BrowserError[] => { + const causeIsGqlError = + hasPopulatedGqlFields(currentError) && + currentError.cause !== undefined && + currentError.cause !== null && + hasPopulatedGqlFields(currentError.cause) + const cause = causeIsGqlError ? currentError.cause : undefined + + errors.push(currentError) + + if (cause !== undefined && cause !== null) { + return flattenErrorsToArray(cause, errors) + } + + return errors + } + + const errors = gqlErrorsAndNotificationsEnabled + ? flattenErrorsToArray(cloneDeep(error)) + : [error] + return errors.reverse() +} diff --git a/src/browser/modules/Stream/CypherFrame/warningUtils.test.ts b/src/browser/modules/Stream/CypherFrame/warningUtils.test.ts index a9fd5a9075e..c65e02e6829 100644 --- a/src/browser/modules/Stream/CypherFrame/warningUtils.test.ts +++ b/src/browser/modules/Stream/CypherFrame/warningUtils.test.ts @@ -23,12 +23,9 @@ import { notificationCategory, notificationSeverityLevel } from 'neo4j-driver-core' -import { - formatSummaryFromGqlStatusObjects, - formatSummaryFromNotifications -} from './warningUtilts' +import { formatSummary } from './warningUtilts' -describe('format rseult summary', () => { +describe('format result summary', () => { test('formats result summary for notifications', () => { const resultSummary = { server: { @@ -63,7 +60,7 @@ describe('format rseult summary', () => { ] } - const result = formatSummaryFromNotifications(resultSummary) + const result = formatSummary(resultSummary, false) expect(result).toEqual([ { @@ -139,7 +136,7 @@ describe('format rseult summary', () => { gqlStatusObjects } - const result = formatSummaryFromGqlStatusObjects(resultSummary) + const result = formatSummary(resultSummary, true) expect(result).toEqual([ { diff --git a/src/browser/modules/Stream/CypherFrame/warningUtilts.ts b/src/browser/modules/Stream/CypherFrame/warningUtilts.ts index df7872ee223..fa25c502f11 100644 --- a/src/browser/modules/Stream/CypherFrame/warningUtilts.ts +++ b/src/browser/modules/Stream/CypherFrame/warningUtilts.ts @@ -75,20 +75,21 @@ const mapNotificationsToFormattedNotifications = ( const SEVERITY_LEVELS = ['ERROR', 'WARNING', 'INFORMATION'] -export const hasPopulatedGqlFields = ( - resultSummary: Partial +const hasPopulatedGqlFields = ( + resultSummary: Partial | undefined ): resultSummary is Partial & { gqlStatusObjects: GqlStatusObject[] notifications: Notification[] } => { return ( + resultSummary !== undefined && 'gqlStatusObjects' in resultSummary && resultSummary.gqlStatusObjects !== undefined ) } -export const formatSummaryFromNotifications = ( - resultSummary?: Partial +const formatSummaryFromNotifications = ( + resultSummary: Partial | undefined ): FormattedNotification[] => { const filteredNotifications = resultSummary?.notifications?.filter(x => @@ -97,8 +98,8 @@ export const formatSummaryFromNotifications = ( return mapNotificationsToFormattedNotifications(filteredNotifications) } -export const formatSummaryFromGqlStatusObjects = ( - resultSummary?: Partial +const formatSummaryFromGqlStatusObjects = ( + resultSummary: Partial | undefined ): FormattedNotification[] => { const filteredStatusObjects = resultSummary?.gqlStatusObjects?.filter(x => @@ -106,3 +107,13 @@ export const formatSummaryFromGqlStatusObjects = ( ) ?? [] return mapGqlStatusObjectsToFormattedNotifications(filteredStatusObjects) } + +export const formatSummary = ( + resultSummary: Partial | undefined, + gqlErrorsAndNotificationsEnabled: boolean +): FormattedNotification[] => { + return hasPopulatedGqlFields(resultSummary) && + gqlErrorsAndNotificationsEnabled + ? formatSummaryFromGqlStatusObjects(resultSummary) + : formatSummaryFromNotifications(resultSummary) +} diff --git a/src/browser/modules/Stream/styled.tsx b/src/browser/modules/Stream/styled.tsx index 8fa26d88aa3..b6bc7e51d64 100644 --- a/src/browser/modules/Stream/styled.tsx +++ b/src/browser/modules/Stream/styled.tsx @@ -49,10 +49,8 @@ export const DottedLineHover = styled.span` text-overflow: ellipsis; ` -export const StyledHelpFrame = styled.div<{ nested?: boolean }>` - padding: 0 30px 30px 0; - padding-top: ${props => (props.nested ? '0' : '30px')}; - padding-left: ${props => (props.nested ? '80px' : '30px')}; +export const StyledHelpFrame = styled.div` + padding: 0 30px 30px 30px; ` export const StyledHelpContent = styled.div` padding-top: 10px; diff --git a/src/shared/modules/features/versionedFeatures.ts b/src/shared/modules/features/versionedFeatures.ts index 187e9b121dd..8b924095b76 100644 --- a/src/shared/modules/features/versionedFeatures.ts +++ b/src/shared/modules/features/versionedFeatures.ts @@ -32,6 +32,8 @@ export const FIRST_MULTI_DB_SUPPORT = NEO4J_4_0 // compatible bolt server. export const FIRST_NO_MULTI_DB_SUPPORT = '3.4.0' +export const FIRST_GQL_ERRORS_NOTIFICATIONS_SUPPORT = '5.27.0' + export const getShowCurrentUserProcedure = (serverVersion: string) => { const serverVersionGuessed = guessSemverVersion(serverVersion) diff --git a/src/shared/services/gqlUtils.ts b/src/shared/services/gqlUtils.ts new file mode 100644 index 00000000000..23cee664005 --- /dev/null +++ b/src/shared/services/gqlUtils.ts @@ -0,0 +1,11 @@ +import { gte } from 'semver' +import { GlobalState } from 'shared/globalState' +import { getSemanticVersion } from 'shared/modules/dbMeta/dbMetaDuck' +import { FIRST_GQL_ERRORS_NOTIFICATIONS_SUPPORT } from 'shared/modules/features/versionedFeatures' + +export const gqlErrorsAndNotificationsEnabled = ( + state: GlobalState +): boolean => { + const version = getSemanticVersion(state) + return version ? gte(version, FIRST_GQL_ERRORS_NOTIFICATIONS_SUPPORT) : false +}