Skip to content

CONSOLE-5012: Create shared error modal launcher using React Context #15946

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.

Sign up for GitHub

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

Open
rhamilto wants to merge 1 commit into
base: main
Choose a base branch
from
+258 −37
Open

CONSOLE-5012: Create shared error modal launcher using React Context #15946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions frontend/packages/console-shared/src/utils/__mocks__/error-modal-handler.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Mock implementation of error-modal-handler for Jest tests
*/

export const mockLaunchErrorModal = jest.fn();

export const SyncErrorModalLauncher = () => null;

export const useErrorModalLauncher = jest.fn(() => mockLaunchErrorModal);

export const launchErrorModal = mockLaunchErrorModal;
99 changes: 99 additions & 0 deletions frontend/packages/console-shared/src/utils/__tests__/error-modal-handler.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { render } from '@testing-library/react';
import {
SyncErrorModalLauncher,
useErrorModalLauncher,
launchErrorModal,
} from '../error-modal-handler';

// Mock useOverlay
const mockLauncher = jest.fn();
jest.mock('@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay', () => ({
useOverlay: () => mockLauncher,
}));

describe('error-modal-handler', () => {
beforeEach(() => {
jest.clearAllMocks();
});

describe('SyncErrorModalLauncher', () => {
it('should sync the launcher on mount', () => {
render(<SyncErrorModalLauncher />);

// Call the module-level function
launchErrorModal({ error: 'Test error', title: 'Test' });

// Should have called the mocked overlay launcher
expect(mockLauncher).toHaveBeenCalledWith(expect.anything(), {
error: 'Test error',
title: 'Test',
});
});

it('should cleanup launcher on unmount', () => {
const { unmount } = render(<SyncErrorModalLauncher />);

unmount();

// Should log error instead of crashing
const consoleError = jest.spyOn(console, 'error').mockImplementation();
launchErrorModal({ error: 'Test error' });

expect(consoleError).toHaveBeenCalledWith(
expect.stringContaining('Error modal launcher not initialized'),
expect.any(Object),
);

consoleError.mockRestore();
});
});

describe('useErrorModalLauncher', () => {
it('should return a function that launches error modals', () => {
let capturedLauncher: any;

const TestComponent = () => {
capturedLauncher = useErrorModalLauncher();
return null;
};

render(<TestComponent />);

capturedLauncher({ error: 'Test error', title: 'Test Title' });

expect(mockLauncher).toHaveBeenCalledWith(expect.anything(), {
error: 'Test error',
title: 'Test Title',
});
});
});

describe('launchErrorModal', () => {
it('should launch error modal when launcher is initialized', () => {
render(<SyncErrorModalLauncher />);

launchErrorModal({
error: 'Connection failed',
title: 'Network Error',
});

expect(mockLauncher).toHaveBeenCalledWith(expect.anything(), {
error: 'Connection failed',
title: 'Network Error',
});
});

it('should log error when launcher is not initialized', () => {
const consoleError = jest.spyOn(console, 'error').mockImplementation();

launchErrorModal({ error: 'Test error' });

expect(consoleError).toHaveBeenCalledWith(
expect.stringContaining('Error modal launcher not initialized'),
{ error: 'Test error' },
);

consoleError.mockRestore();
});
});
});
99 changes: 99 additions & 0 deletions frontend/packages/console-shared/src/utils/error-modal-handler.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { useCallback, useEffect } from 'react';
import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay';
import { ErrorModal, ErrorModalProps } from '@console/internal/components/modals/error-modal';

// Module-level reference for non-React contexts
// This is populated by SyncErrorModalLauncher and should not be set directly
let moduleErrorModalLauncher: ((props: ErrorModalProps) => void) | null = null;

/**
* Component that syncs the error modal launcher to module-level for non-React contexts.
Copy link
Contributor

@vojtechszocs vojtechszocs Feb 3, 2026

Choose a reason for hiding this comment

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

Can we expect to eventually remove all uses of error modal which are outside of React render flow?

i.e. do we have lots of cases where the error modal is used outside of a React component?

Copy link
Member Author

@rhamilto rhamilto Feb 3, 2026

Choose a reason for hiding this comment

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

Claude says

# Error Modal Non-React Usage Report

## Summary

This report documents all locations where error modals are used outside React contexts in the OpenShift Console codebase. These usages justify the hybrid approach with both `useErrorModalLauncher()` hook (for React components) and `launchErrorModal()` function (for non-React contexts).

**Total: 10 non-React usage locations across 7 files**

## Detailed Breakdown

### 1. Topology - Move Node to Group (2 usages)

**Files:**
- `packages/topology/src/utils/moveNodeToGroup.tsx:62`
- `packages/topology/src/utils/moveNodeToGroup.tsx:76`

**Context:**
- Used in `.catch()` callbacks within promise chains
- Called from drag-and-drop `end` event handler (non-React context)
- Executes when moving nodes between application groups fails

**Code Example:**
```typescript
updateTopologyResourceApplication(node, targetGroup?.getLabel())
  .catch((err) => {
    const error = err.message;
    if (onError) {
      onError(error);
    } else {
      launchErrorModal({ error });
    }
  });

Why Non-React Context?

  • Callback from PatternFly Topology drag source spec
  • Executes outside React render cycle
  • No access to hooks

2. Topology - Create Connection

File: packages/topology/src/components/graph-view/components/componentUtils.ts:350

Context:

  • Used in .catch() callback when creating visual connectors
  • Called from createVisualConnector() utility function
  • Triggered during drag-and-drop connection creation

Code Example:

createConnection(source, target, null).catch((error) => {
  launchErrorModal({
    title: i18next.t('topology~Error creating connection'),
    error: error.message,
  });
});

Why Non-React Context?

  • Called from drag-and-drop callback
  • Utility function, not a React component
  • Asynchronous promise chain

3. Topology - Move Edge Connection

File: packages/topology/src/components/graph-view/components/componentUtils.ts:313

Context:

  • Used in edge drag source spec end handler
  • Executes when moving connector edges between nodes

Code Example:

callback(edge.getSource(), dropResult, edge.getTarget()).catch((error) => {
  launchErrorModal({ title, error: error.message });
});

Why Non-React Context?

  • Drag source spec callback (imperative API)
  • Outside React component lifecycle
  • No hook access

4. Topology - Delete Edge

File: packages/topology/src/actions/edgeActions.ts:106

Context:

  • Used in confirmation modal's onConfirm callback
  • Executes after user confirms edge deletion
  • Promise chain error handler

Code Example:

onConfirm: () => {
  return removeTopologyResourceConnection(element, resource).catch((err) => {
    if (err) {
      launchErrorModal({
        title: t('topology~Error deleting connector'),
        error: err.message,
      });
    }
  });
}

Why Non-React Context?

  • Modal callback (outside normal React flow)
  • Asynchronous operation
  • Error handling in promise chain

5. Knative - Event Source Sink Connection

File: packages/knative-plugin/src/topology/components/knativeComponentUtils.ts:191

Context:

  • Edge drag source spec for event sources
  • Moves event source sink connections

Code Example:

createSinkConnection(edge.getSource(), dropResult).catch((error) => {
  launchErrorModal({
    title: i18next.t('knative-plugin~Error moving event source sink'),
    error: error.message,
  });
});

Why Non-React Context?

  • Drag source end handler
  • Imperative callback API
  • No React context available

6. Knative - Kafka Connection (Drag)

File: packages/knative-plugin/src/topology/components/knativeComponentUtils.ts:224

Context:

  • Edge drag source spec for Kafka connections
  • Handles moving Kafka connector edges

Code Example:

createEventSourceKafkaConnection(edge.getSource(), dropResult).catch((error) => {
  launchErrorModal({
    title: i18next.t('knative-plugin~Error moving event source kafka connector'),
    error: error?.message,
  });
});

Why Non-React Context?

  • Drag source callback
  • Outside React render
  • Asynchronous operation

7. Knative - PubSub Connection

File: packages/knative-plugin/src/topology/components/knativeComponentUtils.ts:261

Context:

  • Edge drag source spec for PubSub connections
  • Error handling for sink PubSub connections

Code Example:

createSinkPubSubConnection(edge, dropResult).catch((error) => {
  launchErrorModal({
    title: i18next.t('knative-plugin~Error while sink'),
    error: error.message,
  });
});

Why Non-React Context?

  • Drag source spec callback
  • Imperative API
  • No hook support

8. Knative - Create Kafka Connection (Utility)

File: packages/knative-plugin/src/topology/create-connector-utils.ts:18

Context:

  • Utility function for creating Kafka connections
  • Called from topology connector creation flow

Code Example:

const createKafkaConnection = (source: Node, target: Node) =>
  createEventSourceKafkaConnection(source, target)
    .then(() => null)
    .catch((error) => {
      launchErrorModal({
        title: i18next.t('knative-plugin~Error moving event source kafka connector'),
        error: error.message,
      });
    });

Why Non-React Context?

  • Pure utility function
  • Called from multiple contexts
  • Cannot use React hooks

9. Shipwright - Download Logs

File: packages/shipwright-plugin/src/components/logs/logs-utils.ts:84

Context:

  • Async utility function for downloading build run logs
  • Called by React components but error handling is in utility layer

Code Example:

export const getBuildRunPodLogs = (buildRun: TaskRunKind): Promise<ContainerLogOptions[]> => {
  return k8sGet<PodKind>(PodModel, podName, ns)
    .then((pod) => {
      return /* process logs */;
    })
    .catch((err) => {
      launchErrorModal({
        error: err.message || i18next.t('shipwright-plugin~Error downloading logs.'),
      });
      return [];
    });
};

Why Non-React Context?

  • Async utility function
  • Promise chain error handler
  • Shared by multiple components

10. Dev Console - Expose Route

File: packages/dev-console/src/components/pipeline-section/pipeline/pipeline-template-utils.ts:931

Context:

  • Async utility for exposing webhook routes
  • Error handling in try/catch block

Code Example:

export const exposeWebhookRoute = async (/* params */) => {
  try {
    const route = createRoute(/* params */);
    await k8sCreate(RouteModel, route, { ns });
  } catch (e) {
    launchErrorModal({
      title: 'Error Exposing Route',
      error: e.message || 'Unknown error exposing the Webhook route',
    });
  }
};

Why Non-React Context?

  • Async utility function
  • Try/catch error handling
  • Called from multiple contexts

Usage Categories

By Type

Category Count Percentage
Drag/Drop Operations 7 70%
Async Utility Functions 2 20%
User-Triggered Actions 1 10%
Total 10 100%

By Package

Package Count Files
topology 4 3 files
knative-plugin 4 2 files
shipwright-plugin 1 1 file
dev-console 1 1 file
Total 10 7 files

Can These Be Refactored to React Contexts?

Short Answer: No, most cannot be easily refactored.

Detailed Analysis

Drag/Drop Operations (7/10 usages)

  • ❌ Cannot refactor - These are event handlers from PatternFly's topology library
  • Execute outside React's render cycle
  • Callbacks in imperative third-party APIs
  • No access to React hooks or context

Async Utility Functions (2/10 usages)

  • ❌ Should not refactor - These are pure utility functions
  • Called from multiple contexts (some React, some not)
  • Maintaining as utilities provides better separation of concerns
  • Would require passing error handlers through multiple layers

Modal Callbacks (1/10 usage)

  • ⚠️ Difficult to refactor - Confirmation modal callbacks
  • Execute outside normal React flow
  • Would require restructuring modal architecture

Why the Hybrid Approach is Necessary

The codebase requires both patterns:

useErrorModalLauncher() Hook (React Context)

✅ Use in React components
✅ Direct access to useOverlay()
✅ Type-safe, idiomatic React

launchErrorModal() Function (Module-Level)

✅ Use in imperative callbacks
✅ Use in async utility functions
✅ Use in third-party library callbacks
✅ Use in promise chains

Architecture Benefits

  1. Single Source of Truth: Both APIs use the same underlying OverlayProvider
  2. Proper Lifecycle: SyncErrorModalLauncher manages initialization/cleanup
  3. No Duplication: Shared implementation across all contexts
  4. Type Safety: Both APIs use the same ErrorModalProps interface
  5. Testable: Mock implementation supports both patterns

Conclusion

The current implementation correctly addresses a real architectural need:

  • 10 locations require non-React error modal launching
  • 70% are drag/drop operations (imperative callbacks)
  • Cannot be easily refactored to pure React patterns without major architectural changes

The hybrid approach with SyncErrorModalLauncher + module-level reference is the appropriate solution for this codebase's requirements.

Related

  • PR: #15946
  • Jira: CONSOLE-5012
  • Implementation: packages/console-shared/src/utils/error-modal-handler.tsx

Report generated: 2026-02-03

* This should be mounted once in the app root, after OverlayProvider.
*
* @example
* ```tsx
* const App = () => (
* <OverlayProvider>
* <SyncErrorModalLauncher />
* <YourApp />
* </OverlayProvider>
* );
* ```
*/
export const SyncErrorModalLauncher = () => {
const launcher = useOverlay();

useEffect(() => {
moduleErrorModalLauncher = (props: ErrorModalProps) => {
launcher<ErrorModalProps>(ErrorModal, props);
};

return () => {
moduleErrorModalLauncher = null;
};
}, [launcher]);

return null;
};

/**
* Hook to launch error modals from React components.
* Must be used within an OverlayProvider.
*
* @example
* ```tsx
* const MyComponent = () => {
* const launchErrorModal = useErrorModalLauncher();
*
* const handleError = (error: Error) => {
* launchErrorModal({
* title: 'Operation Failed',
* error: error.message,
* });
* };
*
* // ...
* };
* ```
*/
export const useErrorModalLauncher = (): ((props: ErrorModalProps) => void) => {
const launcher = useOverlay();

return useCallback(
(props: ErrorModalProps) => {
launcher<ErrorModalProps>(ErrorModal, props);
},
[launcher],
);
};

/**
* Launch an error modal from non-React contexts (callbacks, promises, utilities).
* The SyncErrorModalLauncher component must be mounted in the app root.
*
* @deprecated Use React component modals within component code instead.
* For new code, write modals directly within React components using useOverlay or other modal patterns.
* This function should only be used for legacy non-React contexts like promise callbacks.
*
* @example
* ```tsx
* // In a promise callback or utility function
* createConnection(source, target).catch((error) => {
* launchErrorModal({
* title: 'Connection Failed',
* error: error.message,
* });
* });
* ```
*/
export const launchErrorModal = (props: ErrorModalProps): void => {
if (moduleErrorModalLauncher) {
moduleErrorModalLauncher(props);
} else {
// eslint-disable-next-line no-console
console.error(
'Error modal launcher not initialized. Ensure SyncErrorModalLauncher is mounted after OverlayProvider.',
props,
);
}
};
1 change: 1 addition & 0 deletions ...onsole/src/components/pipeline-section/pipeline/__tests__/pipeline-template-utils.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
jest.mock('@console/internal/module/k8s', () => ({
k8sCreate: jest.fn(),
k8sUpdate: jest.fn(),
referenceForModel: jest.fn((model) => model?.kind || 'UnknownModel'),
}));

const getDefaultLabel = (name: string) => ({
Expand Down
4 changes: 2 additions & 2 deletions .../packages/dev-console/src/components/pipeline-section/pipeline/pipeline-template-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import * as _ from 'lodash';
import { compare, gte, parse, SemVer } from 'semver';
import { k8sGet, k8sList, k8sListResourceItems } from '@console/dynamic-plugin-sdk/src/utils/k8s';
import { getActiveUserName } from '@console/internal/actions/ui';
import { errorModal } from '@console/internal/components/modals/error-modal';
import {
ClusterServiceVersionModel,
RouteModel,
Expand All @@ -28,6 +27,7 @@ import {
NameValueFromPair,
NameValuePair,
} from '@console/shared/src/components/formik-fields/field-types';
import { launchErrorModal } from '@console/shared/src/utils/error-modal-handler';
import { getRandomChars } from '@console/shared/src/utils/utils';
import { TektonResourceLabel } from '@console/shipwright-plugin/src/components/logs/TektonTaskRunLog';
import {
Expand Down Expand Up @@ -928,7 +928,7 @@ export const exposeRoute = async (elName: string, ns: string, iteration = 0) =>
);
await k8sCreate(RouteModel, route, { ns });
} catch (e) {
errorModal({
launchErrorModal({
title: 'Error Exposing Route',
error: e.message || 'Unknown error exposing the Webhook route',
});
Expand Down
11 changes: 4 additions & 7 deletions frontend/packages/knative-plugin/src/topology/components/knativeComponentUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
isGraph,
} from '@patternfly/react-topology';
import i18next from 'i18next';
import { errorModal } from '@console/internal/components/modals';
import { launchErrorModal } from '@console/shared/src/utils/error-modal-handler';
import {
NodeComponentProps,
NODE_DRAG_TYPE,
Expand Down Expand Up @@ -188,10 +188,9 @@ export const eventSourceLinkDragSourceSpec = (): DragSourceSpec<
canDropEventSourceSinkOnNode(monitor.getOperation().type, edge, dropResult)
) {
createSinkConnection(edge.getSource(), dropResult).catch((error) => {
errorModal({
launchErrorModal({
title: i18next.t('knative-plugin~Error moving event source sink'),
error: error.message,
showIcon: true,
});
});
}
Expand Down Expand Up @@ -222,10 +221,9 @@ export const eventSourceKafkaLinkDragSourceSpec = (): DragSourceSpec<
edge.setEndPoint();
if (monitor.didDrop() && dropResult) {
createEventSourceKafkaConnection(edge.getSource(), dropResult).catch((error) => {
errorModal({
launchErrorModal({
title: i18next.t('knative-plugin~Error moving event source kafka connector'),
error: error?.message,
showIcon: true,
});
});
}
Expand Down Expand Up @@ -260,10 +258,9 @@ export const eventingPubSubLinkDragSourceSpec = (): DragSourceSpec<
canDropPubSubSinkOnNode(monitor.getOperation().type, edge, dropResult)
) {
createSinkPubSubConnection(edge, dropResult).catch((error) => {
errorModal({
launchErrorModal({
title: i18next.t('knative-plugin~Error while sink'),
error: error.message,
showIcon: true,
});
});
}
Expand Down
5 changes: 2 additions & 3 deletions frontend/packages/knative-plugin/src/topology/create-connector-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Node } from '@patternfly/react-topology/src/types';
import i18next from 'i18next';
import { errorModal } from '@console/internal/components/modals';
import { launchErrorModal } from '@console/shared/src/utils/error-modal-handler';
import { getResource } from '@console/topology/src/utils';
import { addPubSubConnectionModal } from '../components/pub-sub/PubSubModalLauncher';
import { createEventSourceKafkaConnection } from './knative-topology-utils';
Expand All @@ -15,10 +15,9 @@ const createKafkaConnection = (source: Node, target: Node) =>
createEventSourceKafkaConnection(source, target)
.then(() => null)
.catch((error) => {
errorModal({
launchErrorModal({
title: i18next.t('knative-plugin~Error moving event source kafka connector'),
error: error.message,
showIcon: true,
});
});

Expand Down
6 changes: 4 additions & 2 deletions frontend/packages/shipwright-plugin/src/components/logs/logs-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { saveAs } from 'file-saver';
import i18next from 'i18next';
import * as _ from 'lodash';
import { coFetchText } from '@console/internal/co-fetch';
import { errorModal } from '@console/internal/components/modals';
import {
LOG_SOURCE_RESTARTING,
LOG_SOURCE_RUNNING,
Expand All @@ -18,6 +17,7 @@ import {
resourceURL,
k8sGet,
} from '@console/internal/module/k8s';
import { launchErrorModal } from '@console/shared/src/utils/error-modal-handler';
import { TaskRunKind } from '../../types';
import { ComputedStatus, SucceedConditionReason } from './log-snippet-types';
import { getTaskRunLog } from './tekton-results';
Expand Down Expand Up @@ -81,7 +81,9 @@ const getOrderedStepsFromPod = (name: string, ns: string): Promise<ContainerStat
);
})
.catch((err) => {
errorModal({ error: err.message || i18next.t('shipwright-plugin~Error downloading logs.') });
launchErrorModal({
error: err.message || i18next.t('shipwright-plugin~Error downloading logs.'),
});
return [];
});
};
Expand Down
1 change: 1 addition & 0 deletions frontend/packages/topology/locales/en/topology.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"Delete Connector?": "Delete Connector?",
"Deleting the visual connector removes the `connects-to` annotation from the resources. Are you sure you want to delete the visual connector?": "Deleting the visual connector removes the `connects-to` annotation from the resources. Are you sure you want to delete the visual connector?",
"Delete": "Delete",
"Error deleting connector": "Error deleting connector",
"Delete connector": "Delete connector",
"Edit application grouping": "Edit application grouping",
"View all {{size}}": "View all {{size}}",
Expand Down
9 changes: 7 additions & 2 deletions frontend/packages/topology/src/actions/edgeActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Edge, isNode, Node } from '@patternfly/react-topology';
import i18next from 'i18next';
import { useTranslation } from 'react-i18next';
import { Action, K8sModel } from '@console/dynamic-plugin-sdk';
import { errorModal } from '@console/internal/components/modals';
import { asAccessReview } from '@console/internal/components/utils';
import {
TYPE_EVENT_SOURCE,
Expand All @@ -17,6 +16,7 @@ import {
TYPE_MANAGED_KAFKA_CONNECTION,
} from '@console/knative-plugin/src/topology/const';
import { useWarningModal } from '@console/shared/src/hooks/useWarningModal';
import { launchErrorModal } from '@console/shared/src/utils/error-modal-handler';
import { useMoveConnectionModalLauncher } from '../components/modals/MoveConnectionModal';
import { TYPE_CONNECTS_TO, TYPE_TRAFFIC_CONNECTOR } from '../const';
import { removeTopologyResourceConnection, getResource } from '../utils/topology-utils';
Expand Down Expand Up @@ -102,7 +102,12 @@ export const useDeleteConnectorAction = (
confirmButtonVariant: ButtonVariant.danger,
onConfirm: () => {
return removeTopologyResourceConnection(element, resource).catch((err) => {
err && errorModal({ error: err.message });
if (err) {
launchErrorModal({
title: t('topology~Error deleting connector'),
error: err.message,
});
}
});
},
ouiaId: 'TopologyDeleteConnectorConfirmation',
Expand Down
Loading