Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CB-4090 fix: catch throttle errors #2651

Merged
merged 5 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions webapp/packages/core-blocks/src/CommonDialog/RenameDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ interface IRenameDialogState {
message: string | undefined;
valid: boolean;
payload: RenameDialogPayload;
validate: () => void;
validate: () => Promise<void>;
setMessage: (message: string) => void;
}

Expand Down Expand Up @@ -97,7 +97,7 @@ export const RenameDialog: DialogComponent<RenameDialogPayload, string> = observ
);

useEffect(() => {
state.validate();
state.validate().catch(() => {});
}, [name]);

const errorMessage = state.valid ? ' ' : translate(state.message ?? 'ui_rename_taken_or_invalid');
Expand All @@ -108,7 +108,7 @@ export const RenameDialog: DialogComponent<RenameDialogPayload, string> = observ
<CommonDialogBody>
<Form ref={focusedRef} onSubmit={() => resolveDialog(state.name)}>
<Container center>
<InputField name="name" state={state} error={!state.valid} description={errorMessage} onChange={() => state.validate()}>
<InputField name="name" state={state} error={!state.valid} description={errorMessage} onChange={() => state.validate().catch(() => {})}>
{translate('ui_name') + ':'}
</InputField>
</Container>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,24 +119,23 @@ export class ConnectionExecutionContextResource extends CachedMapResource<string
}

async destroy(contextId: string): Promise<void> {
const context = this.get(contextId);
await this.performUpdate(contextId, [], async () => {
const context = this.get(contextId);

if (!context) {
return;
}
if (!context) {
return;
}

await this.performUpdate(contextId, [], async () => {
await this.graphQLService.sdk.executionContextDestroy({
contextId: context.id,
connectionId: context.connectionId,
projectId: context.projectId,
});
this.onDataOutdated.execute(contextId);
this.delete(contextId);
});

runInAction(() => {
this.markOutdated(); // TODO: should be removed, currently multiple contexts for same connection may change catalog/schema for all contexts of connection
this.delete(contextId);
});
}

Expand Down
2 changes: 0 additions & 2 deletions webapp/packages/plugin-data-viewer/src/ContainerDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,6 @@ export class ContainerDataSource extends ResultSetDataSource<IDataContainerOptio

this.clearError();

await this.closeResults(prevResults);

return this.transformResults(executionContext.context!, response.results, limit);
} catch (exception: any) {
this.error = exception;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ export class DatabaseDataModel<TOptions, TResult extends IDatabaseDataResult = I
this.source.resetData();
}

async dispose(): Promise<void> {
async dispose(keepExecutionContext = false): Promise<void> {
await this.onDispose.execute();
await this.source.dispose();
await this.source.dispose(keepExecutionContext);
}

async requestSaveAction(action: () => Promise<void> | void): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,11 @@ export abstract class DatabaseDataSource<TOptions, TResult extends IDatabaseData
return this;
}

async dispose(): Promise<void> {
async dispose(keepExecutionContext = false): Promise<void> {
await this.cancel();
await this.executionContext?.destroy();
if (!keepExecutionContext) {
await this.executionContext?.destroy();
}
}

abstract request(prevResults: TResult[]): TResult[] | Promise<TResult[]>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,5 @@ export interface IDatabaseDataModel<TOptions = any, TResult extends IDatabaseDat
requestDataPortion: (offset: number, count: number) => Promise<void>;
cancel: () => Promise<void> | void;
resetData: () => void;
dispose: () => Promise<void>;
dispose: (keepExecutionContext?: boolean) => Promise<void>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,5 @@ export interface IDatabaseDataSource<TOptions, TResult extends IDatabaseDataResu
cancel: () => Promise<void> | void;
clearError: () => this;
resetData: () => this;
dispose: () => Promise<void>;
dispose: (keepExecutionContext?: boolean) => Promise<void>;
}
17 changes: 15 additions & 2 deletions webapp/packages/plugin-data-viewer/src/ResultSetDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,26 @@ export abstract class ResultSetDataSource<TOptions> extends DatabaseDataSource<T
return this.totalCountRequestTask;
}

async closeResults(results: IDatabaseResultSet[]): Promise<void> {
setResults(results: IDatabaseResultSet[]): this {
this.closeResults(this.results);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this okay we are not await here? just making sure it is on purpose

return super.setResults(results);
}

async dispose(keepExecutionContext?: boolean): Promise<void> {
if (keepExecutionContext) {
await this.closeResults(this.results);
}
return super.dispose(keepExecutionContext);
}

private async closeResults(results: IDatabaseResultSet[]): Promise<void> {
if (!this.executionContext?.context) {
return;
}

for (const result of results) {
if (result.id === null) {
// TODO: it's better to track that context is closed with subscription
if (result.id === null || result.contextId !== this.executionContext.context.id) {
continue;
}
try {
Expand Down
19 changes: 12 additions & 7 deletions webapp/packages/plugin-projects/src/FolderDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ export const FolderDialog: DialogComponent<FolderDialogPayload, IFolderDialogRes
state.setMessage(message);
}
});
} catch {}
} catch (exception: any) {
valid = false;
state.setMessage(exception.message);
}

if (state.folder === folder && state.value === value && state.projectId === projectId) {
state.valid = valid ?? true;
Expand Down Expand Up @@ -143,14 +146,16 @@ export const FolderDialog: DialogComponent<FolderDialogPayload, IFolderDialogRes
const projectInfoLoader = useResource(FolderDialog, ProjectInfoResource, state.projectId);

async function resolveHandler() {
await state.validate();
if (state.valid) {
resolveDialog({ folder: state.folder, name: state.value, projectId: state.projectId });
}
try {
await state.validate();
if (state.valid) {
resolveDialog({ folder: state.folder, name: state.value, projectId: state.projectId });
}
} catch {}
}

useEffect(() => {
state.validate();
state.validate().catch(() => {});
}, [state.value, state.projectId]);

const errorMessage = state.valid ? ' ' : translate(state.message ?? 'ui_rename_taken_or_invalid');
Expand All @@ -169,7 +174,7 @@ export const FolderDialog: DialogComponent<FolderDialogPayload, IFolderDialogRes
error={!state.valid}
description={errorMessage}
loading={state.validationInProgress}
onChange={() => state.validate()}
onChange={() => state.validate().catch(() => {})}
>
{translate('ui_name') + ':'}
</InputField>
Expand Down
2 changes: 0 additions & 2 deletions webapp/packages/plugin-sql-editor/src/QueryDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,6 @@ export class QueryDataSource<TOptions extends IDataQueryOptions = IDataQueryOpti
return prevResults;
}

this.closeResults(prevResults);

return results;
} catch (exception: any) {
this.error = exception;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,9 @@ export class SqlQueryResultService {

if (isGroupEmpty) {
state.resultGroups.splice(state.resultGroups.indexOf(group), 1);

// TODO: we need to dispose table model, but don't close execution context, so now we only
const model = this.tableViewerStorageService.get(group.modelId);
// model?.dispose();

if (model?.source instanceof ResultSetDataSource) {
model.source.closeResults(model.getResults());
model.cancel();
}
model?.dispose(true);

this.tableViewerStorageService.remove(group.modelId);
}
Expand Down
1 change: 0 additions & 1 deletion webapp/packages/plugin-sql-editor/src/useDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
import { useEffect } from 'react';

import { useObjectRef, useObservableRef } from '@cloudbeaver/core-blocks';
import { useService } from '@cloudbeaver/core-di';

import type { ISqlDataSource } from './SqlDataSource/ISqlDataSource';
Expand Down
Loading