IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-01-15 19:22:34 -06:00
Code Issues Packages Projects Releases Wiki Activity

Explore: handle URLs parse errors (#77784)

Browse Source 
* Explore: handle URLs parse errors

* minor modifications

* tentative fix

* update docs

* omit v0 params from the final url

* Apply suggestions from code review

Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>

* remove safeParseJson, make parse return an object

---------

Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>
This commit is contained in:
Giordano Ricci 2023-11-20 13:54:00 +00:00 committed by GitHub
parent b56d7131bd
commit 53642b60ed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 166 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.betterer.results
View File

@ -1546,8 +1546,7 @@ exports[`better eslint`] = {
"public/app/core/utils/explore.ts:5381": [
[0, 0, 0, "Do not use any type assertions.", "0"],
[0, 0, 0, "Unexpected any. Specify a different type.", "1"],
[0, 0, 0, "Unexpected any. Specify a different type.", "2"],
[0, 0, 0, "Unexpected any. Specify a different type.", "3"]
[0, 0, 0, "Unexpected any. Specify a different type.", "2"]
],
"public/app/core/utils/fetch.ts:5381": [
[0, 0, 0, "Do not use any type assertions.", "0"],

4
docs/sources/explore/_index.md
View File

@ -87,6 +87,10 @@ You can then click on any panel icon in the content outline to navigate to that
When using Explore, the URL in the browser address bar updates as you make changes to the queries. You can share or bookmark this URL.
{{% admonition type="note" %}}
Explore may generate relatively long URLs, some tools, like messaging or videoconferencing apps, may truncate messages to a fixed length. In such cases Explore will display a warning message and load a default state. If you encounter issues when sharing Explore links in such apps, you can generate shortened links. See [Share shortened link](#share-shortened-link) for more information.
{{% /admonition %}}
### Generating Explore URLs from external tools
Because Explore URLs have a defined structure, you can build a URL from external tools and open it in Grafana. The URL structure is:

12
public/app/core/utils/explore.ts
View File

@ -148,18 +148,6 @@ export function buildQueryTransaction(
export const clearQueryKeys: (query: DataQuery) => DataQuery = ({ key, ...rest }) => rest;
export const safeParseJson = (text?: string): any | undefined => {
if (!text) {
return;
}
try {
return JSON.parse(text);
} catch (error) {
console.error(error);
}
};
export const safeStringifyValue = (value: unknown, space?: number) => {
if (value === undefined || value === null) {
return '';

14
public/app/features/explore/hooks/useExplorePageTitle.ts
View File

@ -3,7 +3,6 @@ import { useEffect, useRef } from 'react';
import { NavModel } from '@grafana/data';
import { Branding } from 'app/core/components/Branding/Branding';
import { useNavModel } from 'app/core/hooks/useNavModel';
import { safeParseJson } from 'app/core/utils/explore';
import { getDatasourceSrv } from 'app/features/plugins/datasource_srv';
import { ExploreQueryParams } from 'app/types';
@ -19,8 +18,19 @@ export function useExplorePageTitle(params: ExploreQueryParams) {
return;
}
let panesObject: unknown;
try {
panesObject = JSON.parse(params.panes);
} catch {
return;
}
if (typeof panesObject !== 'object' || panesObject === null) {
return;
}
Promise.allSettled(
Object.values(safeParseJson(params.panes)).map((pane) => {
Object.values(panesObject).map((pane) => {
if (
!pane ||
typeof pane !== 'object' ||

75
public/app/features/explore/hooks/useStateSync/index.ts
View File

@ -4,6 +4,7 @@ import { useEffect, useRef } from 'react';
import { CoreApp, ExploreUrlState, DataSourceApi, toURLRange, EventBusSrv } from '@grafana/data';
import { DataQuery, DataSourceRef } from '@grafana/schema';
import { useGrafana } from 'app/core/context/GrafanaContext';
import { useAppNotification } from 'app/core/copy/appNotification';
import { clearQueryKeys, getLastUsedDatasourceUID } from 'app/core/utils/explore';
import { getDatasourceSrv } from 'app/features/plugins/datasource_srv';
import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource';
@ -31,6 +32,7 @@ export function useStateSync(params: ExploreQueryParams) {
const orgId = useSelector((state) => state.user.orgId);
const prevParams = useRef(params);
const initState = useRef<'notstarted' | 'pending' | 'done'>('notstarted');
const { warning } = useAppNotification();
useEffect(() => {
// This happens when the user navigates to an explore "empty page" while within Explore.
@ -76,8 +78,11 @@ export function useStateSync(params: ExploreQueryParams) {
if (!isEqual(prevParams.current.panes, JSON.stringify(panesQueryParams))) {
// If there's no previous state it means we are mounting explore for the first time,
// in this case we want to replace the URL instead of pushing a new entry to the history.
// If the init state is 'pending' it means explore still hasn't finished initializing. in that case we skip
// pushing a new entry in the history as the first entry will be pushed after initialization.
const replace =
!!prevParams.current.panes && Object.values(prevParams.current.panes).filter(Boolean).length === 0;
(!!prevParams.current.panes && Object.values(prevParams.current.panes).filter(Boolean).length === 0) ||
initState.current === 'pending';
prevParams.current = {
panes: JSON.stringify(panesQueryParams),
@ -96,7 +101,12 @@ export function useStateSync(params: ExploreQueryParams) {
useEffect(() => {
const isURLOutOfSync = prevParams.current?.panes !== params.panes;
const urlState = parseURL(params);
const [urlState, hasParseError] = parseURL(params);
hasParseError &&
warning(
'Could not parse Explore URL',
'The requested URL contains invalid parameters, a default Explore state has been loaded.'
);
async function sync() {
// if navigating the history causes one of the time range to not being equal to all the other ones,
@ -225,42 +235,45 @@ export function useStateSync(params: ExploreQueryParams) {
})
);
const newParams = initializedPanes.reduce(
(acc, { exploreId, state }) => {
return {
...acc,
panes: {
...acc.panes,
[exploreId]: getUrlStateFromPaneState(state),
},
};
},
{
panes: {},
}
);
initState.current = 'done';
const panesObj = initializedPanes.reduce((acc, { exploreId, state }) => {
return {
...acc,
[exploreId]: getUrlStateFromPaneState(state),
};
}, {});
// we need to use partial here beacuse replace doesn't encode the query params.
location.partial(
{
// partial doesn't remove other parameters, so we delete (by setting them to undefined) all the current one before adding the new ones.
...Object.keys(location.getSearchObject()).reduce<Record<string, unknown>>((acc, key) => {
acc[key] = undefined;
return acc;
}, {}),
panes: JSON.stringify(newParams.panes),
schemaVersion: urlState.schemaVersion,
orgId,
},
true
);
const oldQuery = location.getSearchObject();
// we create the default query params from the current URL, omitting all the properties we know should be in the final url.
// This includes params from previous schema versions and 'schemaVersion', 'panes', 'orgId' as we want to replace those.
let defaults: Record<string, unknown> = {};
for (const [key, value] of Object.entries(oldQuery).filter(
([key]) => !['schemaVersion', 'panes', 'orgId', 'left', 'right'].includes(key)
)) {
defaults[key] = value;
}
const searchParams = new URLSearchParams({
// we set the schemaVersion as the first parameter so that when URLs are truncated the schemaVersion is more likely to be present.
schemaVersion: `${urlState.schemaVersion}`,
panes: JSON.stringify(panesObj),
orgId: `${orgId}`,
...defaults,
});
location.replace({
pathname: location.getLocation().pathname,
search: searchParams.toString(),
});
initState.current = 'done';
});
}
prevParams.current = params;
isURLOutOfSync && initState.current === 'done' && sync();
}, [dispatch, panesState, orgId, location, params]);
}, [dispatch, panesState, orgId, location, params, warning]);
}
function getDefaultQuery(ds: DataSourceApi) {

2
public/app/features/explore/hooks/useStateSync/migrators/types.ts
View File

@ -4,7 +4,7 @@ export interface MigrationHandler<From extends BaseExploreURL | never, To> {
/**
* The parse function is used to parse the URL parameters into the state object.
*/
parse: (params: UrlQueryMap) => To;
parse: (params: UrlQueryMap) => { to: To; error: boolean };
/**
* the migrate function takes a state object from the previous schema version and returns a new state object
*/

25
public/app/features/explore/hooks/useStateSync/migrators/v0.test.ts
View File

@ -4,16 +4,8 @@ import { v0Migrator } from './v0';
describe('v0 migrator', () => {
describe('parse', () => {
beforeEach(function () {
jest.spyOn(console, 'error').mockImplementation(() => void 0);
});
afterEach(() => {
jest.restoreAllMocks();
});
it('returns default state on empty string', () => {
expect(v0Migrator.parse({})).toMatchObject({
expect(v0Migrator.parse({}).to).toMatchObject({
left: {
datasource: null,
queries: [],
@ -24,7 +16,7 @@ describe('v0 migrator', () => {
it('returns a valid Explore state from URL parameter', () => {
const paramValue = '{"datasource":"Local","queries":[{"expr":"metric"}],"range":{"from":"now-1h","to":"now"}}';
expect(v0Migrator.parse({ left: paramValue })).toMatchObject({
expect(v0Migrator.parse({ left: paramValue }).to).toMatchObject({
left: {
datasource: 'Local',
queries: [{ expr: 'metric' }],
@ -38,7 +30,7 @@ describe('v0 migrator', () => {
it('returns a valid Explore state from right URL parameter', () => {
const paramValue = '{"datasource":"Local","queries":[{"expr":"metric"}],"range":{"from":"now-1h","to":"now"}}';
expect(v0Migrator.parse({ right: paramValue })).toMatchObject({
expect(v0Migrator.parse({ right: paramValue }).to).toMatchObject({
right: {
datasource: 'Local',
queries: [{ expr: 'metric' }],
@ -52,7 +44,7 @@ describe('v0 migrator', () => {
it('returns a default state from invalid right URL parameter', () => {
const paramValue = 10;
expect(v0Migrator.parse({ right: paramValue })).toMatchObject({
expect(v0Migrator.parse({ right: paramValue }).to).toMatchObject({
right: {
datasource: null,
queries: [],
@ -64,7 +56,7 @@ describe('v0 migrator', () => {
it('returns a valid Explore state from a compact URL parameter', () => {
const paramValue =
'["now-1h","now","Local",{"expr":"metric"},{"ui":[true,true,true,"none"],"__panelsState":{"logs":"1"}}]';
expect(v0Migrator.parse({ left: paramValue })).toMatchObject({
expect(v0Migrator.parse({ left: paramValue }).to).toMatchObject({
left: {
datasource: 'Local',
queries: [{ expr: 'metric' }],
@ -81,21 +73,20 @@ describe('v0 migrator', () => {
it('returns default state on compact URLs with too few segments ', () => {
const paramValue = '["now-1h",{"expr":"metric"},{"ui":[true,true,true,"none"]}]';
expect(v0Migrator.parse({ left: paramValue })).toMatchObject({
expect(v0Migrator.parse({ left: paramValue }).to).toMatchObject({
left: {
datasource: null,
queries: [],
range: DEFAULT_RANGE,
},
});
expect(console.error).toHaveBeenCalledWith('Error parsing compact URL state for Explore.');
});
it('should not return a query for mode in the url', () => {
// Previous versions of Grafana included "Explore mode" in the URL; this should not be treated as a query.
const paramValue =
'["now-1h","now","x-ray-datasource",{"queryType":"getTraceSummaries"},{"mode":"Metrics"},{"ui":[true,true,true,"none"]}]';
expect(v0Migrator.parse({ left: paramValue })).toMatchObject({
expect(v0Migrator.parse({ left: paramValue }).to).toMatchObject({
left: {
datasource: 'x-ray-datasource',
queries: [{ queryType: 'getTraceSummaries' }],
@ -110,7 +101,7 @@ describe('v0 migrator', () => {
it('should return queries if queryType is present in the url', () => {
const paramValue =
'["now-1h","now","x-ray-datasource",{"queryType":"getTraceSummaries"},{"ui":[true,true,true,"none"]}]';
expect(v0Migrator.parse({ left: paramValue })).toMatchObject({
expect(v0Migrator.parse({ left: paramValue }).to).toMatchObject({
left: {
datasource: 'x-ray-datasource',
queries: [{ queryType: 'getTraceSummaries' }],

77
public/app/features/explore/hooks/useStateSync/migrators/v0.ts
View File

@ -1,5 +1,4 @@
import { ExploreUrlState } from '@grafana/data';
import { safeParseJson } from 'app/core/utils/explore';
import { DEFAULT_RANGE } from 'app/features/explore/state/utils';
import { BaseExploreURL, MigrationHandler } from './types';
@ -12,12 +11,51 @@ export interface ExploreURLV0 extends BaseExploreURL {
export const v0Migrator: MigrationHandler<never, ExploreURLV0> = {
parse: (params) => {
// If no params are provided, return the default state without errors
// This means the user accessed the explore page without any params
if (!params.left && !params.right) {
return {
to: {
left: {
datasource: null,
queries: [],
range: {
from: 'now-6h',
to: 'now',
},
},
schemaVersion: 0,
},
error: false,
};
}
let left: ExploreUrlState | undefined;
let right: ExploreUrlState | undefined;
let leftError, rightError: boolean | undefined;
if (typeof params.left === 'string') {
[left, leftError] = parsePaneState(params.left);
}
if (typeof params.right === 'string') {
[right, rightError] = parsePaneState(params.right);
} else if (params.right) {
right = FALLBACK_PANE_VALUE;
rightError = true;
}
if (!left) {
left = FALLBACK_PANE_VALUE;
}
return {
schemaVersion: 0,
left: parseUrlState(typeof params.left === 'string' ? params.left : undefined),
...(params.right && {
right: parseUrlState(typeof params.right === 'string' ? params.right : undefined),
}),
to: {
schemaVersion: 0,
left,
...(right && { right }),
},
error: !!leftError || !!rightError,
};
},
};
@ -32,25 +70,26 @@ enum ParseUrlStateIndex {
SegmentsStart = 3,
}
function parseUrlState(initial: string | undefined): ExploreUrlState {
const parsed = safeParseJson(initial);
const errorResult = {
datasource: null,
queries: [],
range: DEFAULT_RANGE,
};
const FALLBACK_PANE_VALUE: ExploreUrlState = {
datasource: null,
queries: [],
range: DEFAULT_RANGE,
};
if (!parsed) {
return errorResult;
function parsePaneState(initial: string): [ExploreUrlState, boolean] {
let parsed;
try {
parsed = JSON.parse(initial);
} catch {
return [FALLBACK_PANE_VALUE, true];
}
if (!Array.isArray(parsed)) {
return { queries: [], range: DEFAULT_RANGE, ...parsed };
return [{ queries: [], range: DEFAULT_RANGE, ...parsed }, false];
}
if (parsed.length <= ParseUrlStateIndex.SegmentsStart) {
console.error('Error parsing compact URL state for Explore.');
return errorResult;
return [FALLBACK_PANE_VALUE, true];
}
const range = {
@ -62,5 +101,5 @@ function parseUrlState(initial: string | undefined): ExploreUrlState {
const queries = parsedSegments.filter((segment) => !isSegment(segment, 'ui', 'mode', '__panelsState'));
const panelsState = parsedSegments.find((segment) => isSegment(segment, '__panelsState'))?.__panelsState;
return { datasource, queries, range, panelsState };
return [{ datasource, queries, range, panelsState }, false];
}

22
public/app/features/explore/hooks/useStateSync/migrators/v1.test.ts
View File

@ -9,16 +9,8 @@ jest.mock('app/core/utils/explore', () => ({
describe('v1 migrator', () => {
describe('parse', () => {
beforeEach(function () {
jest.spyOn(console, 'error').mockImplementation(() => void 0);
});
afterEach(() => {
jest.restoreAllMocks();
});
it('correctly returns default state when no params are provided', () => {
expect(v1Migrator.parse({})).toMatchObject({
expect(v1Migrator.parse({}).to).toMatchObject({
panes: {
ID: {
datasource: null,
@ -30,7 +22,7 @@ describe('v1 migrator', () => {
});
it('correctly returns default state when panes param is an empty object', () => {
expect(v1Migrator.parse({ panes: '{}' })).toMatchObject({
expect(v1Migrator.parse({ panes: '{}' }).to).toMatchObject({
panes: {
ID: {
datasource: null,
@ -42,7 +34,7 @@ describe('v1 migrator', () => {
});
it('correctly returns default state when panes param is not a valid JSON object', () => {
expect(v1Migrator.parse({ panes: '{a malformed json}' })).toMatchObject({
expect(v1Migrator.parse({ panes: '{a malformed json}' }).to).toMatchObject({
panes: {
ID: {
datasource: null,
@ -51,12 +43,10 @@ describe('v1 migrator', () => {
},
},
});
expect(console.error).toHaveBeenCalledTimes(1);
});
it('correctly returns default state when a pane in panes params is an empty object', () => {
expect(v1Migrator.parse({ panes: '{"aaa": {}}' })).toMatchObject({
expect(v1Migrator.parse({ panes: '{"aaa": {}}' }).to).toMatchObject({
panes: {
aaa: {
datasource: null,
@ -68,7 +58,7 @@ describe('v1 migrator', () => {
});
it('correctly returns default state when a pane in panes params is not a valid JSON object', () => {
expect(v1Migrator.parse({ panes: '{"aaa": "NOT A VALID URL STATE"}' })).toMatchObject({
expect(v1Migrator.parse({ panes: '{"aaa": "NOT A VALID URL STATE"}' }).to).toMatchObject({
panes: {
aaa: {
datasource: null,
@ -96,7 +86,7 @@ describe('v1 migrator', () => {
}
}
}`,
})
}).to
).toMatchObject({
panes: {
aaa: {

35
public/app/features/explore/hooks/useStateSync/migrators/v1.ts
View File

@ -1,5 +1,5 @@
import { ExploreUrlState } from '@grafana/data';
import { generateExploreId, safeParseJson } from 'app/core/utils/explore';
import { generateExploreId } from 'app/core/utils/explore';
import { DEFAULT_RANGE } from 'app/features/explore/state/utils';
import { hasKey } from '../../utils';
@ -18,14 +18,32 @@ export const v1Migrator: MigrationHandler<ExploreURLV0, ExploreURLV1> = {
parse: (params) => {
if (!params || !params.panes || typeof params.panes !== 'string') {
return {
schemaVersion: 1,
panes: {
[generateExploreId()]: DEFAULT_STATE,
to: {
schemaVersion: 1,
panes: {
[generateExploreId()]: DEFAULT_STATE,
},
},
error: false,
};
}
const rawPanes: Record<string, unknown> = safeParseJson(params.panes) || {};
let rawPanes: unknown;
try {
rawPanes = JSON.parse(params.panes);
} catch {}
if (rawPanes == null || typeof rawPanes !== 'object') {
return {
to: {
schemaVersion: 1,
panes: {
[generateExploreId()]: DEFAULT_STATE,
},
},
error: true,
};
}
const panes = Object.entries(rawPanes)
.map(([key, value]) => [key, applyDefaults(value)] as const)
@ -41,8 +59,11 @@ export const v1Migrator: MigrationHandler<ExploreURLV0, ExploreURLV1> = {
}
return {
schemaVersion: 1,
panes,
to: {
schemaVersion: 1,
panes,
},
error: false,
};
},
migrate: (params) => {

8
public/app/features/explore/hooks/useStateSync/parseURL.ts
View File

@ -11,20 +11,20 @@ export const parseURL = (params: ExploreQueryParams) => {
const migrators = [v0Migrator, v1Migrator] as const;
const migrate = (params: ExploreQueryParams): ExploreURL => {
const migrate = (params: ExploreQueryParams): [ExploreURL, boolean] => {
const schemaVersion = getSchemaVersion(params);
const [parser, ...migratorsToRun] = migrators.slice(schemaVersion);
const parsedUrl = parser.parse(params);
const { error, to } = parser.parse(params);
// @ts-expect-error
const final: ExploreURL = migratorsToRun.reduce((acc, migrator) => {
// @ts-expect-error
return migrator.migrate ? migrator.migrate(acc) : acc;
}, parsedUrl);
}, to);
return final;
return [final, error];
};
function getSchemaVersion(params: ExploreQueryParams): number {