From 96b5f8a57321e21b26dd1ff09aa0c18b1d5f0565 Mon Sep 17 00:00:00 2001 From: kchawlani19 Date: Sat, 29 Aug 2026 20:48:21 +0530 Subject: [PATCH] OCPBUGS-114021: Retry failed console plugin manifest fetches A 401 from a console replica that does not have the session, or a transient network error, left the plugin unloaded for the rest of the browser session. Retry the manifest load so the plugin can recover without a full page reload. Fixes https://issues.redhat.com/browse/OCPBUGS-114021 --- .../src/runtime/__tests__/plugin-init.spec.ts | 211 ++++++++++++++++++ .../src/runtime/plugin-init.ts | 91 +++++++- 2 files changed, 291 insertions(+), 11 deletions(-) create mode 100644 frontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-init.spec.ts diff --git a/frontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-init.spec.ts b/frontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-init.spec.ts new file mode 100644 index 00000000000..1829cc152dd --- /dev/null +++ b/frontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-init.spec.ts @@ -0,0 +1,211 @@ +import type { PluginStore } from '@openshift/dynamic-plugin-sdk'; +import { ErrorWithCause } from '../../utils/error/custom-error'; +import { HttpError, TimeoutError } from '../../utils/error/http-error'; +import { + isRetryablePluginManifestError, + loadAndEnablePlugin, + PLUGIN_MANIFEST_MAX_ATTEMPTS, +} from '../plugin-init'; + +type PluginStoreMock = { + loadPlugin: jest.Mock; + getPluginInfo: jest.Mock; + disablePlugins: jest.Mock; +}; + +const createPluginStoreMock = (): PluginStoreMock => ({ + loadPlugin: jest.fn().mockResolvedValue(undefined), + getPluginInfo: jest.fn().mockReturnValue([]), + disablePlugins: jest.fn(), +}); + +const asPluginStore = (store: PluginStoreMock) => store as unknown as PluginStore; + +const loadedPluginInfo = ( + name: string, + options: { disableStaticPlugins?: string[]; registrationMethod?: string } = {}, +) => ({ + status: 'loaded' as const, + manifest: { + name, + registrationMethod: options.registrationMethod ?? 'callback', + customProperties: { + console: { + disableStaticPlugins: options.disableStaticPlugins ?? [], + }, + }, + }, +}); + +const failedPluginInfo = (name: string, errorMessage: string, errorCause?: unknown) => ({ + status: 'failed' as const, + manifest: { name, registrationMethod: 'callback' }, + errorMessage, + errorCause, +}); + +const wrapManifestError = (cause: unknown) => + new ErrorWithCause('Failed to load plugin manifest', cause); + +describe('isRetryablePluginManifestError', () => { + it.each([ + ['401', new HttpError('Unauthorized', 401)], + ['408', new HttpError('Request Timeout', 408)], + ['429', new HttpError('Too Many Requests', 429)], + ['500', new HttpError('Internal Server Error', 500)], + ['502', new HttpError('Bad Gateway', 502)], + ['503', new HttpError('Service Unavailable', 503)], + ['504', new HttpError('Gateway Timeout', 504)], + ['TimeoutError', new TimeoutError('/api/plugins/test/plugin-manifest.json', 60000)], + ['TypeError', new TypeError('Failed to fetch')], + ])('returns true for %s', (_label, err) => { + expect(isRetryablePluginManifestError(err)).toBe(true); + }); + + it('returns true when a retryable error is wrapped as ErrorWithCause', () => { + expect( + isRetryablePluginManifestError(wrapManifestError(new HttpError('Unauthorized', 401))), + ).toBe(true); + }); + + it.each([ + ['400', new HttpError('Bad Request', 400)], + ['403', new HttpError('Forbidden', 403)], + ['404', new HttpError('Not Found', 404)], + ['generic Error', new Error('invalid plugin manifest')], + ])('returns false for %s', (_label, err) => { + expect(isRetryablePluginManifestError(err)).toBe(false); + }); +}); + +describe('loadAndEnablePlugin', () => { + const originalServerFlags = window.SERVER_FLAGS; + let store: PluginStoreMock; + let onError: jest.Mock; + let warnSpy: jest.SpyInstance; + + beforeEach(() => { + store = createPluginStoreMock(); + onError = jest.fn(); + warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + window.SERVER_FLAGS = { ...originalServerFlags, basePath: '/test/' }; + }); + + afterEach(() => { + window.SERVER_FLAGS = originalServerFlags; + warnSpy.mockRestore(); + }); + + it('loads the plugin manifest from {basePath}api/plugins/{pluginName}/plugin-manifest.json', async () => { + store.getPluginInfo.mockReturnValue([loadedPluginInfo('monitoring-plugin')]); + + await loadAndEnablePlugin('monitoring-plugin', asPluginStore(store), onError); + + expect(store.loadPlugin).toHaveBeenCalledTimes(1); + expect(store.loadPlugin).toHaveBeenCalledWith( + 'http://localhost/test/api/plugins/monitoring-plugin/plugin-manifest.json', + ); + expect(onError).not.toHaveBeenCalled(); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('retries a 401 manifest fetch and succeeds on a later attempt', async () => { + store.loadPlugin + .mockRejectedValueOnce(wrapManifestError(new HttpError('Unauthorized', 401))) + .mockResolvedValueOnce(undefined); + store.getPluginInfo.mockReturnValue([loadedPluginInfo('monitoring-plugin')]); + + await loadAndEnablePlugin('monitoring-plugin', asPluginStore(store), onError); + + expect(store.loadPlugin).toHaveBeenCalledTimes(2); + expect(onError).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0][0]).toContain( + '[loadAndEnablePlugin] monitoring-plugin manifest fetch failed (attempt 1/3), retrying', + ); + }); + + it('retries a blocked/network fetch (TypeError) and succeeds on a later attempt', async () => { + store.loadPlugin + .mockRejectedValueOnce(wrapManifestError(new TypeError('Failed to fetch'))) + .mockResolvedValueOnce(undefined); + store.getPluginInfo.mockReturnValue([loadedPluginInfo('monitoring-plugin')]); + + await loadAndEnablePlugin('monitoring-plugin', asPluginStore(store), onError); + + expect(store.loadPlugin).toHaveBeenCalledTimes(2); + expect(onError).not.toHaveBeenCalled(); + }); + + it('gives up after PLUGIN_MANIFEST_MAX_ATTEMPTS 401 failures', async () => { + const unauthorized = wrapManifestError(new HttpError('Unauthorized', 401)); + store.loadPlugin.mockRejectedValue(unauthorized); + + await loadAndEnablePlugin('monitoring-plugin', asPluginStore(store), onError); + + expect(store.loadPlugin).toHaveBeenCalledTimes(PLUGIN_MANIFEST_MAX_ATTEMPTS); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith( + '[loadAndEnablePlugin] monitoring-plugin loadPlugin failed: Failed to load plugin manifest', + unauthorized.cause, + ); + expect(warnSpy).toHaveBeenCalledTimes(PLUGIN_MANIFEST_MAX_ATTEMPTS - 1); + }); + + it('does not retry a non-transient manifest error', async () => { + const notFound = wrapManifestError(new HttpError('Not Found', 404)); + store.loadPlugin.mockRejectedValue(notFound); + + await loadAndEnablePlugin('monitoring-plugin', asPluginStore(store), onError); + + expect(store.loadPlugin).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith( + '[loadAndEnablePlugin] monitoring-plugin loadPlugin failed: Failed to load plugin manifest', + notFound.cause, + ); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('reports a plugin that loaded into failed status without retrying', async () => { + const scriptError = new Error('script error'); + store.getPluginInfo.mockReturnValue([ + failedPluginInfo('monitoring-plugin', 'Failed to load scripts', scriptError), + ]); + + await loadAndEnablePlugin('monitoring-plugin', asPluginStore(store), onError); + + expect(store.loadPlugin).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith( + '[loadAndEnablePlugin] monitoring-plugin loading failed: Failed to load scripts', + scriptError, + ); + }); + + it('disables listed static plugins when the dynamic plugin loads', async () => { + store.getPluginInfo.mockImplementation(() => [ + loadedPluginInfo('monitoring-plugin', { disableStaticPlugins: ['console-app'] }), + loadedPluginInfo('console-app', { registrationMethod: 'local' }), + ]); + + await loadAndEnablePlugin('monitoring-plugin', asPluginStore(store), onError); + + expect(store.disablePlugins).toHaveBeenCalledWith( + ['console-app'], + 'disableStaticPlugins in monitoring-plugin', + ); + expect(onError).not.toHaveBeenCalled(); + }); + + it('does not let a dynamic plugin disable another dynamic plugin', async () => { + store.getPluginInfo.mockImplementation(() => [ + loadedPluginInfo('monitoring-plugin', { disableStaticPlugins: ['other-dynamic'] }), + loadedPluginInfo('other-dynamic', { registrationMethod: 'callback' }), + ]); + + await loadAndEnablePlugin('monitoring-plugin', asPluginStore(store), onError); + + expect(store.disablePlugins).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-init.ts b/frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-init.ts index 5b78cf3e544..d4107d13293 100644 --- a/frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-init.ts +++ b/frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-init.ts @@ -10,30 +10,99 @@ import { dynamicPluginNames } from '@console/plugin-sdk/src/utils/allowed-plugin import { addTestError } from '@console/shared/src/utils/test-errors'; import { REMOTE_ENTRY_CALLBACK } from '../constants'; import type { ErrorWithCause } from '../utils/error/custom-error'; +import { HttpError, TimeoutError } from '../utils/error/http-error'; import { resolveURL } from '../utils/url'; +/** Matches the number of attempts used by `coFetch` for RetryError. */ +export const PLUGIN_MANIFEST_MAX_ATTEMPTS = 3; + +const RETRYABLE_HTTP_STATUS_CODES = new Set([401, 408, 429, 500, 502, 503, 504]); + +/** + * True when a plugin manifest fetch failed for a transient reason. + * + * A 401 is retryable because Console sessions are stored per pod. The first + * request after login can land on a replica that does not have the session + * and return Unauthorized; a later attempt may hit the replica that does. + * + * Walks `cause` so SDK `ErrorWithCause` wrappers around `HttpError` still match. + */ +export const isRetryablePluginManifestError = (err: unknown): boolean => { + const visited = new Set(); + let current: unknown = err; + + while (current != null && !visited.has(current)) { + visited.add(current); + + if (current instanceof HttpError && RETRYABLE_HTTP_STATUS_CODES.has(current.code ?? 0)) { + return true; + } + + if (current instanceof TimeoutError || current instanceof TypeError) { + return true; + } + + current = + typeof current === 'object' && 'cause' in current + ? (current as { cause: unknown }).cause + : undefined; + } + + return false; +}; + /** * Calls {@link PluginStore.loadPlugin} for the given plugin name, and * checks if the plugin was loaded successfully. * * Our `PluginStore` is configured to automatically enable loaded plugins. + * + * Manifest fetch failures that look transient (401 from a replica without a + * session, 5xx, timeouts, network errors) are retried. `PluginStore` does not + * register the plugin until the manifest is fetched, so a retry is a clean + * second attempt rather than a reload of a failed plugin. */ -const loadAndEnablePlugin = async ( +export const loadAndEnablePlugin = async ( pluginName: string, pluginStore: PluginStore, onError: (errorMessage: string, errorCause?: unknown) => void = _.noop, ) => { - await pluginStore - .loadPlugin( - resolveURL( - `${window.SERVER_FLAGS.basePath}api/plugins/${pluginName}/`, - 'plugin-manifest.json', - ), - ) - .catch((err: ErrorWithCause) => { + const manifestURL = resolveURL( + `${window.SERVER_FLAGS.basePath}api/plugins/${pluginName}/`, + 'plugin-manifest.json', + ); + + let lastError: { message: string; cause?: unknown } | undefined; + + for (let attempt = 1; attempt <= PLUGIN_MANIFEST_MAX_ATTEMPTS; attempt++) { + try { + // eslint-disable-next-line no-await-in-loop -- sequential retries of a single plugin load + await pluginStore.loadPlugin(manifestURL); + lastError = undefined; + break; + } catch (err) { // ErrorWithCause isn't the exact type but it's close enough for our use - onError(`[loadAndEnablePlugin] ${pluginName} loadPlugin failed: ${err.message}`, err.cause); - }); + const error = err as ErrorWithCause; + lastError = { message: error.message, cause: error.cause }; + + if (!isRetryablePluginManifestError(error) || attempt === PLUGIN_MANIFEST_MAX_ATTEMPTS) { + break; + } + + // eslint-disable-next-line no-console + console.warn( + `[loadAndEnablePlugin] ${pluginName} manifest fetch failed (attempt ${attempt}/${PLUGIN_MANIFEST_MAX_ATTEMPTS}), retrying`, + error.cause ?? error, + ); + } + } + + if (lastError) { + onError( + `[loadAndEnablePlugin] ${pluginName} loadPlugin failed: ${lastError.message}`, + lastError.cause, + ); + } const plugin = pluginStore.getPluginInfo().find((p) => p.manifest.name === pluginName);