Skip to content

Commit

Permalink
fix: DH-17292 Handle disconnect from GridWidgetPlugin (#2086)
Browse files Browse the repository at this point in the history
- When the model is disconnected, we should just display an error.
There's no option to reconnect, as the widget schema could change
entirely
- By unloading the IrisGrid component, it's no longer throwing an error
by trying to access table methods after a table.close()
- Fixes DH-17292 from Enterprise
  - Tested using the steps in the description
  • Loading branch information
mofojed committed Aug 27, 2024
1 parent 336e1f3 commit 0a924cd
Show file tree
Hide file tree
Showing 4 changed files with 302 additions and 85 deletions.
50 changes: 21 additions & 29 deletions packages/dashboard-core-plugins/src/GridWidgetPlugin.tsx
Original file line number Diff line number Diff line change
@@ -1,42 +1,34 @@
import { useEffect, useState } from 'react';
import { type WidgetComponentProps } from '@deephaven/plugin';
import { type dh } from '@deephaven/jsapi-types';
import { useApi } from '@deephaven/jsapi-bootstrap';
import {
IrisGrid,
IrisGridModelFactory,
type IrisGridModel,
} from '@deephaven/iris-grid';
import { IrisGrid } from '@deephaven/iris-grid';
import { useSelector } from 'react-redux';
import { getSettings, RootState } from '@deephaven/redux';
import { LoadingOverlay } from '@deephaven/components';
import { getErrorMessage } from '@deephaven/utils';
import { useIrisGridModel } from './useIrisGridModel';

export function GridWidgetPlugin(
props: WidgetComponentProps<dh.Table>
): JSX.Element | null {
const dh = useApi();
export function GridWidgetPlugin({
fetch,
}: WidgetComponentProps<dh.Table>): JSX.Element | null {
const settings = useSelector(getSettings<RootState>);
const [model, setModel] = useState<IrisGridModel>();

const { fetch } = props;
const fetchResult = useIrisGridModel(fetch);

useEffect(() => {
let cancelled = false;
async function init() {
const table = await fetch();
const newModel = await IrisGridModelFactory.makeModel(dh, table);
if (!cancelled) {
setModel(newModel);
}
}
if (fetchResult.status === 'loading') {
return <LoadingOverlay isLoading />;
}

init();
if (fetchResult.status === 'error') {
return (
<LoadingOverlay
errorMessage={getErrorMessage(fetchResult.error)}
isLoading={false}
/>
);
}

return () => {
cancelled = true;
};
}, [dh, fetch]);

return model ? <IrisGrid model={model} settings={settings} /> : null;
const { model } = fetchResult;
return <IrisGrid model={model} settings={settings} />;
}

export default GridWidgetPlugin;
82 changes: 26 additions & 56 deletions packages/dashboard-core-plugins/src/PandasWidgetPlugin.tsx
Original file line number Diff line number Diff line change
@@ -1,64 +1,34 @@
import { useCallback, useEffect, useState } from 'react';
import { WidgetComponentProps } from '@deephaven/plugin';
import { type dh } from '@deephaven/jsapi-types';
import IrisGrid, {
IrisGridModelFactory,
type IrisGridModel,
} from '@deephaven/iris-grid';
import { useApi } from '@deephaven/jsapi-bootstrap';
import IrisGrid from '@deephaven/iris-grid';
import { LoadingOverlay } from '@deephaven/components';
import { getErrorMessage } from '@deephaven/utils';
import { PandasReloadButton } from './panels/PandasReloadButton';

export function PandasWidgetPlugin(
props: WidgetComponentProps<dh.Table>
): JSX.Element | null {
const dh = useApi();
const [model, setModel] = useState<IrisGridModel>();
const [isLoading, setIsLoading] = useState(true);
const [isLoaded, setIsLoaded] = useState(false);

const { fetch } = props;

const makeModel = useCallback(async () => {
const table = await fetch();
return IrisGridModelFactory.makeModel(dh, table);
}, [dh, fetch]);

const handleReload = useCallback(async () => {
setIsLoading(true);
const newModel = await makeModel();
setModel(newModel);
setIsLoading(false);
}, [makeModel]);

useEffect(() => {
let cancelled = false;
async function init() {
const newModel = await makeModel();
if (!cancelled) {
setModel(newModel);
setIsLoaded(true);
setIsLoading(false);
}
}

init();
setIsLoading(true);

return () => {
cancelled = true;
};
}, [makeModel]);

import { useIrisGridModel } from './useIrisGridModel';

export function PandasWidgetPlugin({
fetch,
}: WidgetComponentProps<dh.Table>): JSX.Element | null {
const fetchResult = useIrisGridModel(fetch);

if (fetchResult.status === 'loading') {
return <LoadingOverlay isLoading />;
}

if (fetchResult.status === 'error') {
return (
<LoadingOverlay
errorMessage={getErrorMessage(fetchResult.error)}
isLoading={false}
/>
);
}

const { model, reload } = fetchResult;
return (
<>
<LoadingOverlay isLoaded={isLoaded} isLoading={isLoading} />
{model && (
<IrisGrid model={model}>
<PandasReloadButton onClick={handleReload} />
</IrisGrid>
)}
</>
<IrisGrid model={model}>
<PandasReloadButton onClick={reload} />
</IrisGrid>
);
}

Expand Down
136 changes: 136 additions & 0 deletions packages/dashboard-core-plugins/src/useIrisGridModel.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import { IrisGridModel } from '@deephaven/iris-grid';
import { type dh } from '@deephaven/jsapi-types';
import { TestUtils } from '@deephaven/utils';
import { renderHook } from '@testing-library/react-hooks';
import { act } from 'react-test-renderer';
import {
IrisGridModelFetchErrorResult,
IrisGridModelFetchSuccessResult,
useIrisGridModel,
} from './useIrisGridModel';

const mockApi = TestUtils.createMockProxy<typeof dh>();
// Mock out the useApi hook to just return the API
jest.mock('@deephaven/jsapi-bootstrap', () => ({
useApi: () => mockApi,
}));

const mockModel = TestUtils.createMockProxy<IrisGridModel>();
// Mock out the IrisGridModelFactory as well
jest.mock('@deephaven/iris-grid', () => ({
...jest.requireActual('@deephaven/iris-grid'),
IrisGridModelFactory: {
makeModel: jest.fn(() => mockModel),
},
}));

it('should return loading status while fetching', () => {
const fetch = jest.fn(
() =>
new Promise<dh.Table>(() => {
// Do nothing
})
);
const { result } = renderHook(() => useIrisGridModel(fetch));
expect(result.current.status).toBe('loading');
});

it('should return error status on fetch error', async () => {
const error = new Error('Test error');
const fetch = jest.fn(() => Promise.reject(error));
const { result, waitForNextUpdate } = renderHook(() =>
useIrisGridModel(fetch)
);
await waitForNextUpdate();
const fetchResult = result.current;
expect(fetchResult.status).toBe('error');
expect((fetchResult as IrisGridModelFetchErrorResult).error).toBe(error);
});

it('should return success status on fetch success', async () => {
const table = TestUtils.createMockProxy<dh.Table>();
const fetch = jest.fn(() => Promise.resolve(table));
const { result, waitForNextUpdate } = renderHook(() =>
useIrisGridModel(fetch)
);
await waitForNextUpdate();
const fetchResult = result.current;
expect(fetchResult.status).toBe('success');
expect((fetchResult as IrisGridModelFetchSuccessResult).model).toBeDefined();
});

it('should reload the model on reload', async () => {
const table = TestUtils.createMockProxy<dh.Table>();
let fetchResolve;
const fetch = jest.fn(
() =>
new Promise<dh.Table>(resolve => {
fetchResolve = resolve;
})
);
const { result, waitForNextUpdate } = renderHook(() =>
useIrisGridModel(fetch)
);
expect(result.current.status).toBe('loading');
fetchResolve(table);
await waitForNextUpdate();
expect(result.current.status).toBe('success');
// Check that it will reload, transitioning to loading then to success again

fetch.mockClear();
fetch.mockReturnValue(
new Promise(resolve => {
fetchResolve = resolve;
})
);
await act(async () => {
result.current.reload();
});
expect(fetch).toHaveBeenCalledTimes(1);
expect(result.current.status).toBe('loading');
fetchResolve(table);
await waitForNextUpdate();
expect(result.current.status).toBe('success');
expect(
(result.current as IrisGridModelFetchSuccessResult).model
).toBeDefined();

// Now check that it will handle a failure on reload, transitioning from loading to failure
fetch.mockClear();

let fetchReject;
fetch.mockReturnValue(
new Promise((resolve, reject) => {
fetchReject = reject;
})
);
await act(async () => {
result.current.reload();
});
expect(fetch).toHaveBeenCalledTimes(1);
expect(result.current.status).toBe('loading');
const error = new Error('Test error');
fetchReject(error);
await waitForNextUpdate();
expect(result.current.status).toBe('error');
expect((result.current as IrisGridModelFetchErrorResult).error).toBe(error);

// Check that it will reload again after an error
fetch.mockClear();
fetch.mockReturnValue(
new Promise(resolve => {
fetchResolve = resolve;
})
);
await act(async () => {
result.current.reload();
});
expect(fetch).toHaveBeenCalledTimes(1);
expect(result.current.status).toBe('loading');
fetchResolve(table);
await waitForNextUpdate();
expect(result.current.status).toBe('success');
expect(
(result.current as IrisGridModelFetchSuccessResult).model
).toBeDefined();
});
Loading

0 comments on commit 0a924cd

Please sign in to comment.