From 1b0182b0a72a642a4d2b12bfa42bdc3e7b0c86f7 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Wed, 22 Jan 2025 17:11:55 +0100 Subject: [PATCH 1/5] added error handling --- packages/base/src/commands.ts | 2 +- packages/base/src/mainview/mainView.tsx | 21 ++++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/base/src/commands.ts b/packages/base/src/commands.ts index 647e1c4a4..cc8153112 100644 --- a/packages/base/src/commands.ts +++ b/packages/base/src/commands.ts @@ -1042,7 +1042,7 @@ namespace Private { itemTypeToRemove: SelectionType, removeFunction: (id: string) => void ) { - const selected = model?.localState?.selected.value; + const selected = model?.localState?.selected?.value; if (!selected) { console.info('Nothing selected'); diff --git a/packages/base/src/mainview/mainView.tsx b/packages/base/src/mainview/mainView.tsx index 38b114482..f8209292b 100644 --- a/packages/base/src/mainview/mainView.tsx +++ b/packages/base/src/mainview/mainView.tsx @@ -82,6 +82,7 @@ import CollaboratorPointers, { ClientPointer } from './CollaboratorPointers'; import { FollowIndicator } from './FollowIndicator'; import { MainViewModel } from './mainviewmodel'; import { Spinner } from './spinner'; +import { showErrorMessage } from '@jupyterlab/apputils'; interface IProps { viewModel: MainViewModel; @@ -646,7 +647,7 @@ export class MainView extends React.Component { break; } } - + newSource.set('id', id); // _sources is a list of OpenLayers sources this._sources[id] = newSource; @@ -944,11 +945,21 @@ export class MainView extends React.Component { return; } - const newMapLayer = await this._buildMapLayer(id, layer); - if (newMapLayer !== undefined) { - await this._waitForReady(); + try { + const newMapLayer = await this._buildMapLayer(id, layer); + if (newMapLayer !== undefined) { + await this._waitForReady(); - this._Map.getLayers().insertAt(index, newMapLayer); + this._Map.getLayers().insertAt(index, newMapLayer); + } + } catch (error) { + await showErrorMessage( + `Error Adding ${layer.name}`, + `Failed to add ${layer.name}. Please remove it` + ); + this.setState(old => ({ ...old, loadingLayer: false })); + this._loadingLayers.delete(id); + this._model.removeLayer(id); } } From f6d3bef84141e91ffd666e5c21070c8aacf22b10 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Tue, 28 Jan 2025 11:51:05 +0100 Subject: [PATCH 2/5] modify error msg. --- packages/base/src/mainview/mainView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/base/src/mainview/mainView.tsx b/packages/base/src/mainview/mainView.tsx index f8209292b..a4c5bf1a1 100644 --- a/packages/base/src/mainview/mainView.tsx +++ b/packages/base/src/mainview/mainView.tsx @@ -955,7 +955,7 @@ export class MainView extends React.Component { } catch (error) { await showErrorMessage( `Error Adding ${layer.name}`, - `Failed to add ${layer.name}. Please remove it` + `Failed to add ${layer.name}.` ); this.setState(old => ({ ...old, loadingLayer: false })); this._loadingLayers.delete(id); From 119be7c2d91bb3de5485f4abfc60d152f8a609a3 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Wed, 29 Jan 2025 17:20:04 +0100 Subject: [PATCH 3/5] store invalid layers+errors --- packages/base/src/mainview/mainView.tsx | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/base/src/mainview/mainView.tsx b/packages/base/src/mainview/mainView.tsx index a4c5bf1a1..975916e58 100644 --- a/packages/base/src/mainview/mainView.tsx +++ b/packages/base/src/mainview/mainView.tsx @@ -100,6 +100,7 @@ interface IStates { viewProjection: { code: string; units: string }; loadingLayer: boolean; scale: number; + loadingErrors: Array<{ id: string; error: any; index: number }>; } export class MainView extends React.Component { @@ -139,7 +140,8 @@ export class MainView extends React.Component { clientPointers: {}, viewProjection: { code: '', units: '' }, loadingLayer: false, - scale: 0 + scale: 0, + loadingErrors: [] }; this._sources = []; @@ -950,16 +952,24 @@ export class MainView extends React.Component { if (newMapLayer !== undefined) { await this._waitForReady(); - this._Map.getLayers().insertAt(index, newMapLayer); + // Adjust index to ensure it's within bounds + const numLayers = this._Map.getLayers().getLength(); + const safeIndex = Math.min(index, numLayers); + this._Map.getLayers().insertAt(safeIndex, newMapLayer); } } catch (error) { + if (this.state.loadingErrors.find(item => item.id === id)) { + this._loadingLayers.delete(id); + return; + } + await showErrorMessage( `Error Adding ${layer.name}`, `Failed to add ${layer.name}.` ); this.setState(old => ({ ...old, loadingLayer: false })); + this.state.loadingErrors.push({ id, error, index }); this._loadingLayers.delete(id); - this._model.removeLayer(id); } } @@ -1419,7 +1429,11 @@ export class MainView extends React.Component { if (currentIndex < index) { nextIndex -= 1; } - this._Map.getLayers().insertAt(nextIndex, layer); + // Adjust index to ensure it's within bounds + const numLayers = this._Map.getLayers().getLength(); + const safeIndex = Math.min(index, numLayers); + + this._Map.getLayers().insertAt(safeIndex, layer); } /** From 77862264107badad7d7e058843c41c7148af2447 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Wed, 29 Jan 2025 17:38:06 +0100 Subject: [PATCH 4/5] include error message in the dialog. --- packages/base/src/mainview/mainView.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/base/src/mainview/mainView.tsx b/packages/base/src/mainview/mainView.tsx index 975916e58..7b07255ec 100644 --- a/packages/base/src/mainview/mainView.tsx +++ b/packages/base/src/mainview/mainView.tsx @@ -649,7 +649,7 @@ export class MainView extends React.Component { break; } } - + newSource.set('id', id); // _sources is a list of OpenLayers sources this._sources[id] = newSource; @@ -735,6 +735,9 @@ export class MainView extends React.Component { targetLayerPosition++ ) { const layerId = layerIds[targetLayerPosition]; + if (this.state.loadingErrors.find(item => item.id === layerId)) { + continue; + } const layer = this._model.sharedModel.getLayer(layerId); if (!layer) { @@ -957,7 +960,7 @@ export class MainView extends React.Component { const safeIndex = Math.min(index, numLayers); this._Map.getLayers().insertAt(safeIndex, newMapLayer); } - } catch (error) { + } catch (error: any) { if (this.state.loadingErrors.find(item => item.id === id)) { this._loadingLayers.delete(id); return; @@ -965,7 +968,7 @@ export class MainView extends React.Component { await showErrorMessage( `Error Adding ${layer.name}`, - `Failed to add ${layer.name}.` + `Failed to add ${layer.name}: ${error.message || 'invalid file path.'}` ); this.setState(old => ({ ...old, loadingLayer: false })); this.state.loadingErrors.push({ id, error, index }); From 461d402224033ed0e0d12c9c000962f27b0b4c52 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Thu, 30 Jan 2025 16:12:39 +0100 Subject: [PATCH 5/5] compare error message. --- packages/base/src/mainview/mainView.tsx | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/base/src/mainview/mainView.tsx b/packages/base/src/mainview/mainView.tsx index 7b07255ec..a1d59933a 100644 --- a/packages/base/src/mainview/mainView.tsx +++ b/packages/base/src/mainview/mainView.tsx @@ -735,9 +735,6 @@ export class MainView extends React.Component { targetLayerPosition++ ) { const layerId = layerIds[targetLayerPosition]; - if (this.state.loadingErrors.find(item => item.id === layerId)) { - continue; - } const layer = this._model.sharedModel.getLayer(layerId); if (!layer) { @@ -961,17 +958,25 @@ export class MainView extends React.Component { this._Map.getLayers().insertAt(safeIndex, newMapLayer); } } catch (error: any) { - if (this.state.loadingErrors.find(item => item.id === id)) { + if ( + this.state.loadingErrors.find( + item => item.id === id && item.error === error.message + ) + ) { this._loadingLayers.delete(id); return; } await showErrorMessage( `Error Adding ${layer.name}`, - `Failed to add ${layer.name}: ${error.message || 'invalid file path.'}` + `Failed to add ${layer.name}: ${error.message || 'invalid file path'}` ); this.setState(old => ({ ...old, loadingLayer: false })); - this.state.loadingErrors.push({ id, error, index }); + this.state.loadingErrors.push({ + id, + error: error.message || 'invalid file path', + index + }); this._loadingLayers.delete(id); } }