Skip to content

Commit

Permalink
Data Explorer: Display duckdb error message when file fails to open (#…
Browse files Browse the repository at this point in the history
…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 <brianlambert@gmail.com>
  • Loading branch information
wesm and softwarenerd authored Oct 31, 2024
1 parent 9e0a108 commit e0d844b
Show file tree
Hide file tree
Showing 10 changed files with 211 additions and 43 deletions.
81 changes: 69 additions & 12 deletions extensions/positron-duckdb/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -255,12 +259,22 @@ export class DuckDBTableView {

constructor(readonly uri: string, readonly tableName: string,
readonly fullSchema: Array<SchemaEntry>,
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<TableSchema> {
return {
columns: params.column_indices.map((index) => {
Expand Down Expand Up @@ -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<BackendState> {
if (!this.isConnected) {
return this.getDisconnectedState();
}

const [unfiltedNumRows, unfilteredNumCols] = await this._unfilteredShape;
const [filteredNumRows, filteredNumCols] = await this._filteredShape;
return {
Expand Down Expand Up @@ -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 {};
}

Expand Down
13 changes: 13 additions & 0 deletions extensions/positron-duckdb/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export interface DataExplorerResponse {
// AUTO-GENERATED from data_explorer.json; do not edit.
//


/**
* Result in Methods
*/
Expand Down Expand Up @@ -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;

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
8 changes: 8 additions & 0 deletions positron/comms/data_explorer-backend-openrpc.json
Original file line number Diff line number Diff line change
Expand Up @@ -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."
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,55 +12,63 @@
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;
padding: 8px 16px;
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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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<PositronDataExplorerClosedProps>) => {
// Constants.
export const PositronDataExplorerClosed = (
props: PropsWithChildren<PositronDataExplorerClosedProps>
) => {
// 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"
Expand All @@ -36,15 +67,14 @@ export const PositronDataExplorerClosed = (props: PropsWithChildren<PositronData
// Render.
return (
<div className='positron-data-explorer-closed'>
<div className='message' >
<div>
{(() => localize(
'positron.dataExplorerEditor.thisObjectIsNoLongerAvailable',
'This object is no longer available.'
))()}
<div className='dialog-box' >
<div className='message'>
{message}
</div>
<div className='error-message'>
{errorMessage}
</div>
<Button
className='close-button'
<Button className='close-button'
ariaLabel={closeDataExplorer}
onPressed={props.onClose}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { PositronActionBarServices } from 'vs/platform/positronActionBar/browser
import { PositronDataExplorerContextProvider } from 'vs/workbench/browser/positronDataExplorer/positronDataExplorerContext';
import { DataExplorerPanel } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerPanel/dataExplorerPanel';
import { IPositronDataExplorerInstance } from 'vs/workbench/services/positronDataExplorer/browser/interfaces/positronDataExplorerInstance';
import { PositronDataExplorerClosed } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed';
import { PositronDataExplorerClosed, PositronDataExplorerClosedStatus } from 'vs/workbench/browser/positronDataExplorer/components/dataExplorerClosed/positronDataExplorerClosed';

/**
* PositronDataExplorerServices interface.
Expand All @@ -45,6 +45,12 @@ export interface PositronDataExplorerProps extends PositronDataExplorerConfigura
onClose: () => void;
}

export enum PositronDataExplorerUiStatus {
OPEN = 'open',
UNAVAILABLE = 'unavailable',
ERROR = 'error'
}

/**
* PositronDataExplorer component.
* @param props A PositronDataExplorerProps that contains the component properties.
Expand All @@ -53,6 +59,8 @@ export interface PositronDataExplorerProps extends PositronDataExplorerConfigura
export const PositronDataExplorer = (props: PropsWithChildren<PositronDataExplorerProps>) => {
// State hooks.
const [closed, setClosed] = useState(false);
const [reason, setReason] = useState(PositronDataExplorerClosedStatus.UNAVAILABLE);
const [errorMessage, setErrorMessage] = useState('');

// Main useEffect.
useEffect(() => {
Expand All @@ -64,6 +72,16 @@ export const PositronDataExplorer = (props: PropsWithChildren<PositronDataExplor
setClosed(true);
}));

disposableStore.add(props.instance.dataExplorerClientInstance.onDidUpdateBackendState((state) => {
if (state.connected === false) {
setClosed(true);
if (state.error_message) {
setReason(PositronDataExplorerClosedStatus.ERROR);
setErrorMessage(state.error_message);
}
}
}));

// Return the cleanup function that will dispose of the event handlers.
return () => disposableStore.dispose();
}, [props.instance]);
Expand All @@ -74,7 +92,13 @@ export const PositronDataExplorer = (props: PropsWithChildren<PositronDataExplor
<div className='positron-data-explorer'>
<ActionBar />
<DataExplorerPanel />
{closed && <PositronDataExplorerClosed onClose={props.onClose} />}
{closed && (
<PositronDataExplorerClosed
closedReason={reason}
errorMessage={errorMessage}
onClose={props.onClose}
/>
)}
</div>
</PositronDataExplorerContextProvider>
);
Expand Down
Loading

0 comments on commit e0d844b

Please sign in to comment.