From e0d844b031f95acbf89f234a2cce2af9b6721f6c Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 30 Oct 2024 19:45:42 -0500 Subject: [PATCH] Data Explorer: Display duckdb error message when file fails to open (#5172) Addresses #5133. This was a bit messy but the simplest way I saw to bubble up the error information. I added optional parameters to BackendState that allow an error message to be intercepted and displayed. ![image](https://github.com/user-attachments/assets/9945add9-b469-4329-ae15-34f979620c54) I need some CSS help to make the div a reasonable relative width and for the text to wrap. --------- Co-authored-by: Brian Lambert --- extensions/positron-duckdb/src/extension.ts | 81 ++++++++++++++++--- extensions/positron-duckdb/src/interfaces.ts | 13 +++ .../positron_ipykernel/data_explorer_comm.py | 10 +++ .../comms/data_explorer-backend-openrpc.json | 8 ++ .../positronDataExplorerClosed.css | 44 +++++----- .../positronDataExplorerClosed.tsx | 50 +++++++++--- .../positronDataExplorer.tsx | 28 ++++++- .../browser/positronDataExplorerEditor.tsx | 3 +- .../languageRuntimeDataExplorerClient.ts | 5 ++ .../common/positronDataExplorerComm.ts | 12 +++ 10 files changed, 211 insertions(+), 43 deletions(-) diff --git a/extensions/positron-duckdb/src/extension.ts b/extensions/positron-duckdb/src/extension.ts index 206ba5eca1c..cff64dd147f 100644 --- a/extensions/positron-duckdb/src/extension.ts +++ b/extensions/positron-duckdb/src/extension.ts @@ -94,7 +94,11 @@ class DuckDBInstance { } return result; } catch (error) { - return JSON.stringify(error); + if (error instanceof Error) { + return error.message; + } else { + return JSON.stringify(error); + } } } @@ -255,12 +259,22 @@ export class DuckDBTableView { constructor(readonly uri: string, readonly tableName: string, readonly fullSchema: Array, - readonly db: DuckDBInstance + readonly db: DuckDBInstance, + readonly isConnected: boolean = true, + readonly errorMessage: string = '' ) { - this._unfilteredShape = this._getShape(); + if (isConnected) { + this._unfilteredShape = this._getShape(); + } else { + this._unfilteredShape = Promise.resolve([0, 0]); + } this._filteredShape = this._unfilteredShape; } + static getDisconnected(uri: string, errorMessage: string, db: DuckDBInstance) { + return new DuckDBTableView(uri, 'disconnected', [], db, false, errorMessage); + } + async getSchema(params: GetSchemaParams): RpcResponse { return { columns: params.column_indices.map((index) => { @@ -477,7 +491,50 @@ END`; return 'not implemented'; } + private getDisconnectedState(): BackendState { + return { + display_name: this.uri, + connected: false, + error_message: this.errorMessage, + table_shape: { num_rows: 0, num_columns: 0 }, + table_unfiltered_shape: { num_rows: 0, num_columns: 0 }, + has_row_labels: false, + column_filters: [], + row_filters: [], + sort_keys: [], + supported_features: { + search_schema: { + support_status: SupportStatus.Unsupported, + supported_types: [] + }, + set_column_filters: { + support_status: SupportStatus.Unsupported, + supported_types: [] + }, + set_row_filters: { + support_status: SupportStatus.Unsupported, + supports_conditions: SupportStatus.Unsupported, + supported_types: [] + }, + get_column_profiles: { + support_status: SupportStatus.Unsupported, + supported_types: [] + }, + set_sort_columns: { support_status: SupportStatus.Unsupported, }, + export_data_selection: { + support_status: SupportStatus.Unsupported, + supported_formats: [] + } + } + }; + + } + async getState(): RpcResponse { + if (!this.isConnected) { + return this.getDisconnectedState(); + } + const [unfiltedNumRows, unfilteredNumCols] = await this._unfilteredShape; const [filteredNumRows, filteredNumCols] = await this._filteredShape; return { @@ -763,19 +820,19 @@ export class DataExplorerRpcHandler { } let result = await this.db.runQuery(scanQuery); + let tableView; if (typeof result === 'string') { - return { error_message: result }; - } + tableView = DuckDBTableView.getDisconnected(params.uri, result, this.db); + } else { + const schemaQuery = `DESCRIBE ${tableName};`; + result = await this.db.runQuery(schemaQuery); + if (typeof result === 'string') { + return { error_message: result }; + } - const schemaQuery = `DESCRIBE ${tableName};`; - result = await this.db.runQuery(schemaQuery); - if (typeof result === 'string') { - return { error_message: result }; + tableView = new DuckDBTableView(params.uri, tableName, result.toArray(), this.db); } - - const tableView = new DuckDBTableView(params.uri, tableName, result.toArray(), this.db); this._uriToTableView.set(params.uri, tableView); - return {}; } diff --git a/extensions/positron-duckdb/src/interfaces.ts b/extensions/positron-duckdb/src/interfaces.ts index 5aee3ba5952..41dd2bea010 100644 --- a/extensions/positron-duckdb/src/interfaces.ts +++ b/extensions/positron-duckdb/src/interfaces.ts @@ -58,6 +58,7 @@ export interface DataExplorerResponse { // AUTO-GENERATED from data_explorer.json; do not edit. // + /** * Result in Methods */ @@ -162,6 +163,18 @@ export interface BackendState { */ supported_features: SupportedFeatures; + /** + * Optional flag allowing backend to report that it is unable to serve + * requests. This parameter may change. + */ + connected?: boolean; + + /** + * Optional experimental parameter to provide an explanation when + * connected=false. This parameter may change. + */ + error_message?: string; + } /** diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py index eee9c7dffb7..78c10c14c92 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/data_explorer_comm.py @@ -302,6 +302,16 @@ class BackendState(BaseModel): description="The features currently supported by the backend instance", ) + connected: Optional[StrictBool] = Field( + default=None, + description="Optional flag allowing backend to report that it is unable to serve requests. This parameter may change.", + ) + + error_message: Optional[StrictStr] = Field( + default=None, + description="Optional experimental parameter to provide an explanation when connected=false. This parameter may change.", + ) + class ColumnSchema(BaseModel): """ diff --git a/positron/comms/data_explorer-backend-openrpc.json b/positron/comms/data_explorer-backend-openrpc.json index f7fc7639ebf..801d3dd8edc 100644 --- a/positron/comms/data_explorer-backend-openrpc.json +++ b/positron/comms/data_explorer-backend-openrpc.json @@ -387,6 +387,14 @@ "supported_features": { "description": "The features currently supported by the backend instance", "$ref": "#/components/schemas/supported_features" + }, + "connected": { + "type": "boolean", + "description": "Optional flag allowing backend to report that it is unable to serve requests. This parameter may change." + }, + "error_message": { + "type": "string", + "description": "Optional experimental parameter to provide an explanation when connected=false. This parameter may change." } } } diff --git a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed.css b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed.css index 1565fd85caf..6fb742a2625 100644 --- a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed.css +++ b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed.css @@ -12,31 +12,41 @@ display: grid; position: absolute; background-color: transparent; - grid-template-rows: [top-gutter] 1fr [message] max-content [bottom-gutter] 1fr [end]; - grid-template-columns: [left-gutter] 1fr [message] max-content [right-gutter] 1fr [end]; + background: rgba(0, 0, 0, 0.05); + grid-template-rows: [top-gutter] 1fr [dialog-box] max-content [bottom-gutter] 1fr [end-rows]; + grid-template-columns: [left-gutter] 1fr [dialog-box] max-content [right-gutter] 1fr [end-columns]; } -.positron-data-explorer-closed -.message { - color: white; +.positron-data-explorer-closed .dialog-box { display: flex; - row-gap: 10px; + row-gap: 12px; font-size: 14px; cursor: pointer; - padding: 25px 50px; + max-width: 300px; + padding: 20px; + align-items: center; border-radius: 6px; - text-align: center; flex-direction: column; - grid-row: message / bottom-gutter; - grid-column: message / right-gutter; + grid-row: dialog-box / bottom-gutter; + grid-column: dialog-box / right-gutter; box-shadow: 0 0 8px 2px var(--vscode-widget-shadow); color: var(--vscode-positronModalDialog-foreground); border: 1px solid var(--vscode-positronModalDialog-border); background-color: var(--vscode-positronModalDialog-background); } -.positron-data-explorer-closed -.close-button { +.positron-data-explorer-closed .dialog-box .message { + font-size: 16px; + font-weight: bold; + text-align: center; +} + +.positron-data-explorer-closed .dialog-box .error-message { + text-align: left; + max-height: 200px; +} + +.positron-data-explorer-closed .dialog-box .close-button { display: flex; font-size: 14px; cursor: pointer; @@ -44,23 +54,21 @@ border-radius: 5px; align-items: center; justify-content: center; + max-width: max-content; border: 1px solid var(--vscode-positronModalDialog-buttonBorder); color: var(--vscode-positronModalDialog-defaultButtonForeground); background: var(--vscode-positronModalDialog-defaultButtonBackground); } -.positron-data-explorer-closed -.close-button:hover { +.positron-data-explorer-closed .dialog-box .close-button:hover { background: var(--vscode-positronModalDialog-defaultButtonHoverBackground); } -.positron-data-explorer-closed -.close-button:focus { +.positron-data-explorer-closed .dialog-box .close-button:focus { outline: none; } -.positron-data-explorer-closed -.close-button:focus-visible { +.positron-data-explorer-closed .dialog-box .close-button:focus-visible { outline-offset: 2px; outline: 1px solid var(--vscode-focusBorder); } diff --git a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed.tsx b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed.tsx index ebf55e6fb6e..d4323256104 100644 --- a/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed.tsx +++ b/src/vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed.tsx @@ -14,10 +14,20 @@ import { PropsWithChildren } from 'react'; // eslint-disable-line no-duplicate-i import { localize } from 'vs/nls'; import { Button } from 'vs/base/browser/ui/positronComponents/button/button'; +/** + * PositronDataExplorerClosedStatus enum. + */ +export enum PositronDataExplorerClosedStatus { + UNAVAILABLE = 'unavailable', + ERROR = 'error' +} + /** * PositronDataExplorerClosedProps interface. */ export interface PositronDataExplorerClosedProps { + closedReason: PositronDataExplorerClosedStatus; + errorMessage?: string; onClose: () => void; } @@ -26,8 +36,29 @@ export interface PositronDataExplorerClosedProps { * @param props A PositronDataExplorerClosedProps that contains the component properties. * @returns The rendered component. */ -export const PositronDataExplorerClosed = (props: PropsWithChildren) => { - // Constants. +export const PositronDataExplorerClosed = ( + props: PropsWithChildren +) => { + // Construct the message and error message. + let message, errorMessage; + if (props.closedReason === PositronDataExplorerClosedStatus.ERROR) { + message = localize( + 'positron.dataExplorerEditor.errorOpeningDataExplorer', + 'Error Opening Data Explorer' + ); + errorMessage = props.errorMessage; + } else { + message = localize( + 'positron.dataExplorerEditor.connectionClosed', + 'Connection Closed' + ); + errorMessage = localize( + 'positron.dataExplorerEditor.objectNoLongerAvailable', + 'This object is no longer available.' + ); + } + + // Localize the close button. const closeDataExplorer = localize( 'positron.dataExplorerEditor.closeDataExplorer', "Close Data Explorer" @@ -36,15 +67,14 @@ export const PositronDataExplorerClosed = (props: PropsWithChildren -
-
- {(() => localize( - 'positron.dataExplorerEditor.thisObjectIsNoLongerAvailable', - 'This object is no longer available.' - ))()} +
+
+ {message} +
+
+ {errorMessage}
-
); diff --git a/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.tsx b/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.tsx index 9a2d9dab04a..8f70b27c4b7 100644 --- a/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.tsx +++ b/src/vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditor.tsx @@ -34,7 +34,7 @@ import { IReactComponentContainer, ISize, PositronReactRenderer } from 'vs/base/ import { PositronDataExplorerUri } from 'vs/workbench/services/positronDataExplorer/common/positronDataExplorerUri'; import { IPositronDataExplorerService } from 'vs/workbench/services/positronDataExplorer/browser/interfaces/positronDataExplorerService'; import { PositronDataExplorerEditorInput } from 'vs/workbench/contrib/positronDataExplorerEditor/browser/positronDataExplorerEditorInput'; -import { PositronDataExplorerClosed } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed'; +import { PositronDataExplorerClosed, PositronDataExplorerClosedStatus } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed'; /** * IPositronDataExplorerEditorOptions interface. @@ -352,6 +352,7 @@ export class PositronDataExplorerEditor extends EditorPane implements IPositronD } else { this._positronReactRenderer.render( this._group.closeEditor(this.input)} /> ); diff --git a/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts b/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts index 1813adaffb7..8d6658e009a 100644 --- a/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts +++ b/src/vs/workbench/services/languageRuntime/common/languageRuntimeDataExplorerClient.ts @@ -304,6 +304,11 @@ export class DataExplorerClientInstance extends Disposable { this.cachedBackendState = await this._backendPromise; this._backendPromise = undefined; + if (this.cachedBackendState.connected === false) { + // Halt more requests from going out + this.status = DataExplorerClientStatus.Disconnected; + } + // Notify listeners this._onDidUpdateBackendStateEmitter.fire(this.cachedBackendState); diff --git a/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts b/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts index 7cfb8299a14..d1b213f995e 100644 --- a/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts +++ b/src/vs/workbench/services/languageRuntime/common/positronDataExplorerComm.ts @@ -115,6 +115,18 @@ export interface BackendState { */ supported_features: SupportedFeatures; + /** + * Optional flag allowing backend to report that it is unable to serve + * requests. This parameter may change. + */ + connected?: boolean; + + /** + * Optional experimental parameter to provide an explanation when + * connected=false. This parameter may change. + */ + error_message?: string; + } /**