upstream/grafana
1
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2026-02-03 20:49:50 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 43

Plugins: add error handling for plugin meta calls (#117111)

Browse source 
* Plugins: add error handling for plugin meta calls

* chore: update after PR feedback
This commit is contained in:
Hugo Häggmark 2026-02-03 06:33:15 +01:00 committed by GitHub
parent 5d02a91d26
commit eebacf0b2d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 531 additions and 100 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
.github/CODEOWNERS vendored
View file

@ -677,6 +677,7 @@ i18next.config.ts @grafana/grafana-frontend-platform
/packages/grafana-runtime/src/utils/toDataQueryError.ts @grafana/grafana-datasources-core-services
/packages/grafana-runtime/src/utils/userStorage* @grafana/plugins-platform-frontend @grafana/grafana-frontend-platform
/packages/grafana-runtime/src/utils/useFavoriteDatasources* @grafana/plugins-platform-frontend
/packages/grafana-runtime/src/utils/getCachedPromise* @grafana/plugins-platform-frontend @grafana/grafana-frontend-platform
# @grafana/schema
/packages/grafana-schema/ @grafana/grafana-app-platform-squad

1
packages/grafana-runtime/src/internal/index.ts
View file

@ -32,3 +32,4 @@ export { initOpenFeature, evaluateBooleanFlag } from './openFeature';
export { getAppPluginMeta, getAppPluginMetas, setAppPluginMetas } from '../services/pluginMeta/apps';
export { useAppPluginMeta, useAppPluginMetas } from '../services/pluginMeta/hooks';
export type { AppPluginMetas } from '../services/pluginMeta/types';
export { getCachedPromise, invalidateCache, setLogger } from '../utils/getCachedPromise';

158
packages/grafana-runtime/src/services/pluginMeta/plugins.test.ts
View file

@ -1,6 +1,8 @@
import { evaluateBooleanFlag } from '../../internal/openFeature';
import { invalidateCache, setLogger } from '../../utils/getCachedPromise';
import { type MonitoringLogger } from '../../utils/logging';
import { clearCache, initPluginMetas } from './plugins';
import { initPluginMetas } from './plugins';
import { v0alpha1Meta } from './test-fixtures/v0alpha1Response';
jest.mock('../../internal/openFeature', () => ({
@ -10,19 +12,31 @@ jest.mock('../../internal/openFeature', () => ({
const evaluateBooleanFlagMock = jest.mocked(evaluateBooleanFlag);
const originalFetch = global.fetch;
let loggerMock: MonitoringLogger;
beforeEach(() => {
jest.clearAllMocks();
invalidateCache();
loggerMock = {
logDebug: jest.fn(),
logError: jest.fn(),
logInfo: jest.fn(),
logMeasurement: jest.fn(),
logWarning: jest.fn(),
};
setLogger(loggerMock);
});
afterEach(() => {
global.fetch = originalFetch;
});
describe('when useMTPlugins toggle is enabled and cache is not initialized', () => {
const originalFetch = global.fetch;
beforeEach(() => {
jest.resetAllMocks();
clearCache();
evaluateBooleanFlagMock.mockReturnValue(true);
});
afterEach(() => {
global.fetch = originalFetch;
});
it('initPluginMetas should call loadPluginMetas and return correct result if response is ok', async () => {
global.fetch = jest.fn().mockResolvedValue({
ok: true,
@ -33,83 +47,76 @@ describe('when useMTPlugins toggle is enabled and cache is not initialized', ()
const response = await initPluginMetas();
expect(response.items).toHaveLength(1);
expect(response.items[0]).toEqual(v0alpha1Meta);
expect(global.fetch).toHaveBeenCalledTimes(1);
expect(global.fetch).toHaveBeenCalledWith('/apis/plugins.grafana.app/v0alpha1/namespaces/default/metas');
});
it('initPluginMetas should call loadPluginMetas and return correct result if response is not ok', async () => {
global.fetch = jest.fn().mockResolvedValue({
ok: false,
status: 404,
statusText: 'Not found',
});
await expect(initPluginMetas()).rejects.toThrow(new Error(`Failed to load plugin metas 404:Not found`));
expect(response.items[0]).toBe(v0alpha1Meta);
expect(global.fetch).toHaveBeenCalledTimes(1);
expect(global.fetch).toHaveBeenCalledWith('/apis/plugins.grafana.app/v0alpha1/namespaces/default/metas');
});
});
describe('when useMTPlugins toggle is enabled and cache is initialized', () => {
const originalFetch = global.fetch;
describe('when useMTPlugins toggle is enabled and errors occur', () => {
beforeEach(() => {
jest.resetAllMocks();
clearCache();
evaluateBooleanFlagMock.mockReturnValue(true);
});
afterEach(() => {
global.fetch = originalFetch;
it('initPluginMetas should log when fetch fails', async () => {
global.fetch = jest
.fn()
.mockResolvedValueOnce({
ok: false,
statusText: 'Internal Server Error',
status: 500,
})
.mockResolvedValue({
ok: true,
status: 200,
json: () => Promise.resolve({ items: [v0alpha1Meta] }),
});
await initPluginMetas();
await initPluginMetas();
await initPluginMetas();
expect(global.fetch).toHaveBeenCalledTimes(2); // first + second (because first throws), third is cached
expect(global.fetch).toHaveBeenCalledWith('/apis/plugins.grafana.app/v0alpha1/namespaces/default/metas');
expect(loggerMock.logError).toHaveBeenCalledTimes(1);
expect(loggerMock.logError).toHaveBeenCalledWith(new Error(`Something failed while resolving a cached promise`), {
message: 'Failed to load plugin metas 500:Internal Server Error',
stack: expect.any(String),
key: 'loadPluginMetas',
});
});
it('initPluginMetas should return cache', async () => {
global.fetch = jest.fn().mockResolvedValue({
ok: true,
status: 200,
json: () => Promise.resolve({ items: [v0alpha1Meta] }),
it('initPluginMetas should log when fetch rejects', async () => {
global.fetch = jest
.fn()
.mockRejectedValueOnce(new Error('Network Error'))
.mockResolvedValue({
ok: true,
status: 200,
json: () => Promise.resolve({ items: [v0alpha1Meta] }),
});
await initPluginMetas();
await initPluginMetas();
await initPluginMetas();
expect(global.fetch).toHaveBeenCalledTimes(2); // first + second (because first throws), third is cached
expect(global.fetch).toHaveBeenCalledWith('/apis/plugins.grafana.app/v0alpha1/namespaces/default/metas');
expect(loggerMock.logError).toHaveBeenCalledTimes(1);
expect(loggerMock.logError).toHaveBeenCalledWith(new Error(`Something failed while resolving a cached promise`), {
message: 'Network Error',
stack: expect.any(String),
key: 'loadPluginMetas',
});
const original = await initPluginMetas();
const cached = await initPluginMetas();
expect(original).toEqual(cached);
expect(global.fetch).toHaveBeenCalledTimes(1);
});
it('initPluginMetas should return inflight promise', async () => {
jest.useFakeTimers();
global.fetch = jest.fn().mockResolvedValue({
ok: true,
status: 200,
json: () => Promise.resolve({ items: [v0alpha1Meta] }),
});
const original = initPluginMetas();
const cached = initPluginMetas();
await jest.runAllTimersAsync();
expect(original).toEqual(cached);
expect(global.fetch).toHaveBeenCalledTimes(1);
});
});
describe('when useMTPlugins toggle is disabled and cache is not initialized', () => {
const originalFetch = global.fetch;
beforeEach(() => {
jest.resetAllMocks();
clearCache();
global.fetch = jest.fn();
evaluateBooleanFlagMock.mockReturnValue(false);
});
afterEach(() => {
global.fetch = originalFetch;
});
it('initPluginMetas should call loadPluginMetas and return correct result if response is ok', async () => {
const response = await initPluginMetas();
@ -119,35 +126,16 @@ describe('when useMTPlugins toggle is disabled and cache is not initialized', ()
});
describe('when useMTPlugins toggle is disabled and cache is initialized', () => {
const originalFetch = global.fetch;
beforeEach(() => {
jest.resetAllMocks();
clearCache();
global.fetch = jest.fn();
evaluateBooleanFlagMock.mockReturnValue(false);
});
afterEach(() => {
global.fetch = originalFetch;
});
it('initPluginMetas should return cache', async () => {
const original = await initPluginMetas();
const cached = await initPluginMetas();
expect(original).toEqual(cached);
expect(global.fetch).not.toHaveBeenCalled();
});
it('initPluginMetas should return inflight promise', async () => {
jest.useFakeTimers();
const original = initPluginMetas();
const cached = initPluginMetas();
await jest.runAllTimersAsync();
expect(original).toEqual(cached);
expect(original).toBe(cached);
expect(global.fetch).not.toHaveBeenCalled();
});
});

17
packages/grafana-runtime/src/services/pluginMeta/plugins.ts
View file

@ -1,10 +1,9 @@
import { config } from '../../config';
import { evaluateBooleanFlag } from '../../internal/openFeature';
import { getCachedPromise } from '../../utils/getCachedPromise';
import type { PluginMetasResponse } from './types';
let initPromise: Promise<PluginMetasResponse> | null = null;
function getApiVersion(): string {
return 'v0alpha1';
}
@ -25,17 +24,5 @@ async function loadPluginMetas(): Promise<PluginMetasResponse> {
}
export function initPluginMetas(): Promise<PluginMetasResponse> {
if (!initPromise) {
initPromise = loadPluginMetas();
}
return initPromise;
}
export function clearCache() {
if (process.env.NODE_ENV !== 'test') {
throw new Error('clearCache() function can only be called from tests.');
}
initPromise = null;
return getCachedPromise(loadPluginMetas, { defaultValue: { items: [] } });
}

297
packages/grafana-runtime/src/utils/getCachedPromise.test.ts Normal file
View file

@ -0,0 +1,297 @@
import { getCachedPromise, invalidateCache, MAX_CACHE_SIZE, setLogger } from './getCachedPromise';
import { MonitoringLogger } from './logging';
const TEST_ASYNC_DELAY = 10;
function simulateOkRequest(): Promise<{ ok: boolean; status: number; statusText: string }> {
return new Promise((resolve) => {
setTimeout(() => resolve({ ok: true, status: 200, statusText: 'ok' }), TEST_ASYNC_DELAY);
});
}
function simulateErrorRequest(): Promise<{ ok: boolean; status: number; statusText: string }> {
return new Promise((_, reject) => {
setTimeout(() => reject(new Error('Network Error')), TEST_ASYNC_DELAY);
});
}
let logger: MonitoringLogger;
beforeEach(() => {
jest.clearAllMocks();
invalidateCache();
logger = {
logDebug: jest.fn(),
logError: jest.fn(),
logInfo: jest.fn(),
logMeasurement: jest.fn(),
logWarning: jest.fn(),
};
setLogger(logger);
});
// heads up that all jest.fn(any function) will get the name 'mockConstructor'
// so when getCachedPromise adds/looks up the cache key it will add/look up 'mockConstructor'
describe('getCachedPromise', () => {
describe('when cache limit is reached', () => {
test('should clear cache', async () => {
const entries = Array.from({ length: MAX_CACHE_SIZE + 1 }, (_, i) => i);
const promises = entries.map((value) => {
const func = async () => value;
const cacheKey = `cache-key-${value}`;
return getCachedPromise(func, { cacheKey });
});
await Promise.all(promises);
// Verify all entries are cached correctly
const expectPromises = entries.map((value) => {
const func = async () => 999;
const cacheKey = `cache-key-${value}`;
return getCachedPromise(func, { cacheKey }).then((v) => expect(v).toBe(value));
});
await Promise.all(expectPromises);
// Add one more to exceed limit
await getCachedPromise(simulateOkRequest);
// Verify that all previous cached are cleared
const expectClearedPromises = entries.map((value) => {
const func = async () => 999;
const cacheKey = `cache-key-${value}`;
return getCachedPromise(func, { cacheKey }).then((v) => expect(v).toBe(999));
});
await Promise.all(expectClearedPromises);
});
});
describe('when called with different functions', () => {
test('should cache each function name separately', async () => {
const otherFunction = async () => 2;
const actual1 = await getCachedPromise(simulateOkRequest);
const actual2 = await getCachedPromise(otherFunction);
expect(actual1).toStrictEqual({ ok: true, status: 200, statusText: 'ok' });
expect(actual2).toBe(2);
});
test('should use cacheKey as key when supplied', async () => {
const otherFunction = async () => 2;
const actual1 = await getCachedPromise(simulateOkRequest);
const actual2 = await getCachedPromise(otherFunction, { cacheKey: 'simulateOkRequest' });
expect(actual1).toStrictEqual({ ok: true, status: 200, statusText: 'ok' });
expect(actual2).toBe(actual1);
});
});
describe('when called with anonymous functions', () => {
test('should throw an error', async () => {
await expect(getCachedPromise(async () => 2)).rejects.toThrow(
`getCachedPromise function must be invoked with a named function or cacheKey`
);
});
test('should not throw an error if a cacheKey is supplied', async () => {
const actual = await getCachedPromise(async () => 2, { cacheKey: 'a-cache-key' });
expect(actual).toBe(2);
});
});
describe('when called without defaultValue and onError', () => {
test('should cache promise correctly', async () => {
const promise = jest.fn(simulateOkRequest);
const promise2 = jest.fn(simulateErrorRequest);
const actual1 = await getCachedPromise(promise);
const actual2 = await getCachedPromise(promise2);
expect(actual1).toBe(actual2);
expect(promise).toHaveBeenCalledTimes(1);
expect(promise2).toHaveBeenCalledTimes(0);
});
test('should return inflight promise', async () => {
const promise = jest.fn(simulateOkRequest);
const promiseReject = jest.fn(simulateErrorRequest);
const promise1 = getCachedPromise(promise);
const promise2 = getCachedPromise(promiseReject);
const [actual1, actual2] = await Promise.all([promise1, promise2]);
expect(actual1).toBe(actual2);
expect(promise).toHaveBeenCalledTimes(1);
});
test('should bubble up errors', async () => {
const promise = jest.fn(simulateErrorRequest);
await expect(getCachedPromise(promise)).rejects.toThrow('Network Error');
expect(promise).toHaveBeenCalledTimes(1);
});
test('should not invalidate cache on errors', async () => {
const promise = jest.fn(simulateErrorRequest);
const promise2 = jest.fn(simulateOkRequest);
await expect(getCachedPromise(promise)).rejects.toThrow('Network Error');
await expect(getCachedPromise(promise2)).rejects.toThrow('Network Error');
expect(promise).toHaveBeenCalledTimes(1);
expect(promise2).toHaveBeenCalledTimes(0);
});
});
describe('when called with defaultValue but without onError', () => {
test('should cache promise correctly', async () => {
const promise = jest.fn(simulateOkRequest);
const promise2 = jest.fn(simulateErrorRequest);
const actual1 = await getCachedPromise(promise, {
defaultValue: { ok: false, status: 500, statusText: 'Internal Server Error' },
});
const actual2 = await getCachedPromise(promise2);
expect(actual1).toBe(actual2);
expect(promise).toHaveBeenCalledTimes(1);
expect(promise2).toHaveBeenCalledTimes(0);
});
test('should return inflight promise', async () => {
const promise = jest.fn(simulateOkRequest);
const promise1 = getCachedPromise(promise, {
defaultValue: { ok: false, status: 500, statusText: 'Internal Server Error' },
});
const promise2 = getCachedPromise(promise, {
defaultValue: { ok: false, status: 500, statusText: 'Internal Server Error' },
});
const [actual1, actual2] = await Promise.all([promise1, promise2]);
expect(actual1).toBe(actual2);
expect(promise).toHaveBeenCalledTimes(1);
});
test('should not bubble up errors but handle them and log errors', async () => {
const promise = jest.fn(simulateErrorRequest);
const actual = await getCachedPromise(promise, {
defaultValue: { ok: false, status: 500, statusText: 'Internal Server Error' },
});
expect(actual).toStrictEqual({ ok: false, status: 500, statusText: 'Internal Server Error' });
expect(promise).toHaveBeenCalledTimes(1);
expect(logger.logError).toHaveBeenCalledTimes(1);
expect(logger.logError).toHaveBeenCalledWith(new Error(`Something failed while resolving a cached promise`), {
stack: expect.any(String),
message: 'Network Error',
key: 'mockConstructor',
});
});
test('should invalidate cache when something errors', async () => {
const promise = jest.fn(simulateErrorRequest);
const promise2 = jest.fn(simulateOkRequest);
const actual1 = await getCachedPromise(promise, {
defaultValue: { ok: false, status: 500, statusText: 'Internal Server Error' },
});
const actual2 = await getCachedPromise(promise2);
expect(actual1).toStrictEqual({ ok: false, status: 500, statusText: 'Internal Server Error' });
expect(actual2).toStrictEqual({ ok: true, status: 200, statusText: 'ok' });
expect(promise).toHaveBeenCalledTimes(1);
expect(promise2).toHaveBeenCalledTimes(1); // because the cache is invalidated on error then promise2 is called
});
});
describe('when called with onError but without defaultValue', () => {
test('should cache promise correctly', async () => {
const promise = jest.fn(simulateOkRequest);
const promise2 = jest.fn(simulateErrorRequest);
const actual1 = await getCachedPromise(promise, {
onError: async () => ({ ok: false, status: 500, statusText: 'Internal Server Error' }),
});
const actual2 = await getCachedPromise(promise2);
expect(actual1).toBe(actual2);
expect(promise).toHaveBeenCalledTimes(1);
expect(promise2).toHaveBeenCalledTimes(0);
});
test('should return inflight promise', async () => {
const promise = jest.fn(simulateOkRequest);
const promise1 = getCachedPromise(promise, {
onError: async () => ({ ok: false, status: 500, statusText: 'Internal Server Error' }),
});
const promise2 = getCachedPromise(promise, {
onError: async () => ({ ok: false, status: 500, statusText: 'Internal Server Error' }),
});
const [actual1, actual2] = await Promise.all([promise1, promise2]);
expect(actual1).toBe(actual2);
expect(promise).toHaveBeenCalledTimes(1);
});
test('should not bubble up errors but call onError callback', async () => {
const promise = jest.fn(simulateErrorRequest);
const onError = jest.fn(() => Promise.resolve({ ok: false, status: 500, statusText: 'Internal Server Error' }));
const actual = await getCachedPromise(promise, { onError });
expect(actual).toStrictEqual({ ok: false, status: 500, statusText: 'Internal Server Error' });
expect(promise).toHaveBeenCalledTimes(1);
expect(onError).toHaveBeenCalledTimes(1);
expect(onError).toHaveBeenCalledWith({ error: new Error('Network Error'), invalidate: expect.any(Function) });
});
test('should invalidate cache when calling invalidate function', async () => {
const promise = jest.fn(simulateErrorRequest);
const promise2 = jest.fn(simulateOkRequest);
const actual1 = await getCachedPromise(promise, {
onError: async ({ error, invalidate }) => {
expect(error).toStrictEqual(new Error('Network Error'));
invalidate();
return { ok: false, status: 500, statusText: 'Network Error' };
},
});
const actual2 = await getCachedPromise(promise2);
expect(actual1).toStrictEqual({ ok: false, status: 500, statusText: 'Network Error' });
expect(actual2).toStrictEqual({ ok: true, status: 200, statusText: 'ok' });
expect(promise).toHaveBeenCalledTimes(1);
expect(promise2).toHaveBeenCalledTimes(1); // because the cache is invalidated on error then promise2 is called
});
test('should not invalidate cache if invalidate function is not called', async () => {
const promise = jest.fn(simulateErrorRequest);
const promise2 = jest.fn(simulateOkRequest);
const actual1 = await getCachedPromise(promise, {
onError: async ({ error }) => {
expect(error).toStrictEqual(new Error('Network Error'));
return { ok: false, status: 500, statusText: 'Network Error' };
},
});
const actual2 = await getCachedPromise(promise2);
expect(actual1).toStrictEqual({ ok: false, status: 500, statusText: 'Network Error' });
expect(actual2).toBe(actual1);
expect(promise).toHaveBeenCalledTimes(1);
expect(promise2).toHaveBeenCalledTimes(0); // because the cache is not invalidated on error
});
});
});

157
packages/grafana-runtime/src/utils/getCachedPromise.ts Normal file
View file

@ -0,0 +1,157 @@
import { LogContext } from '@grafana/faro-web-sdk';
import { createMonitoringLogger, MonitoringLogger } from './logging';
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const cache: Map<string, Promise<any>> = new Map();
export const MAX_CACHE_SIZE = 500;
interface OnErrorArgs {
error: unknown;
invalidate: () => void;
}
type PromiseFunction<T> = () => Promise<T>;
interface CachedPromiseOptions<T> {
cacheKey?: string;
defaultValue?: T;
onError?: (args: OnErrorArgs) => Promise<T>;
}
interface CachePromiseWithoutCatchArgs<T> {
key: string;
promise: PromiseFunction<T>;
}
interface CachePromiseWithDefaultArgs<T> {
key: string;
promise: PromiseFunction<T>;
defaultValue: T;
}
interface CachePromiseWithCallbackArgs<T> {
key: string;
promise: PromiseFunction<T>;
onError: (args: OnErrorArgs) => Promise<T>;
}
interface LogErrorArgs {
error: unknown;
key: string;
}
let logger: MonitoringLogger;
function getLogger() {
if (!logger) {
logger = createMonitoringLogger('get-cached-promise-logs');
}
return logger;
}
export function setLogger(override: MonitoringLogger) {
if (process.env.NODE_ENV !== 'test') {
throw new Error('setLogger function can only be called from tests.');
}
logger = override;
}
function logError({ error, key }: LogErrorArgs): void {
const err = error instanceof Error ? error : new Error(String(error));
const context: LogContext = { message: err.message, key };
if (err.stack) {
context.stack = err.stack;
}
getLogger().logError(new Error(`Something failed while resolving a cached promise`), context);
}
function checkCacheSize() {
if (cache.size <= MAX_CACHE_SIZE) {
return;
}
cache.clear();
}
function addToCache<T>(key: string, cached: Promise<T>) {
checkCacheSize();
cache.set(key, cached);
}
function cachePromiseWithoutCatch<T>({ key, promise }: CachePromiseWithoutCatchArgs<T>): Promise<T> {
const cached = promise();
addToCache(key, cached);
return cached;
}
function cachePromiseWithDefaultValue<T>({ defaultValue, key, promise }: CachePromiseWithDefaultArgs<T>): Promise<T> {
const cached = promise().catch((error) => {
logError({ error, key });
cache.delete(key);
return defaultValue;
});
addToCache(key, cached);
return cached;
}
function cachePromiseWithCallback<T>({ key, promise, onError }: CachePromiseWithCallbackArgs<T>): Promise<T> {
const invalidate = () => cache.delete(key);
const cached = promise().catch((error) => onError({ error, invalidate }));
addToCache(key, cached);
return cached;
}
/**
* This utility function will safely handle concurrent requests for the same resource by caching the promise.
* Caches the result of a promise based on the name of the promise function or cacheKey. If a cached promise exists for the given key,
* it returns the cached promise. Otherwise, it executes the promise function and caches the result.
* It also provides options for handling errors, including returning a defaultValue value or invoking a custom error handler.
* If neither defaultValue nor onError is provided, errors will propagate as usual.
*
* @template T - The type of the resolved promise value
* @param promise - Function that returns the promise to be cached
* @param options - Options object for error behaviors
* @param options.cacheKey - Optional cache key to use as key instead of the function name
* @param options.defaultValue - Optional default value to return if the promise rejects
* @param options.onError - Optional error handler that receives the error and an invalidate function
* @returns A promise that resolves to the cached or newly computed value
*/
export function getCachedPromise<T>(promise: PromiseFunction<T>, options?: CachedPromiseOptions<T>): Promise<T> {
const { cacheKey, defaultValue, onError } = options ?? {};
const key = cacheKey ?? promise.name;
if (!key) {
return Promise.reject(new Error(`getCachedPromise function must be invoked with a named function or cacheKey`));
}
const cached = cache.get(key);
if (cached) {
return cached;
}
if (onError) {
return cachePromiseWithCallback({ key, onError, promise });
}
if (defaultValue !== undefined) {
return cachePromiseWithDefaultValue({ defaultValue, key, promise });
}
return cachePromiseWithoutCatch({ key, promise });
}
export function invalidateCache() {
if (process.env.NODE_ENV !== 'test') {
throw new Error('invalidateCache function can only be called from tests.');
}
cache.clear();
}