Skip to content

Commit e3f24b9

Browse files
authored
fix(dashboards): preserve multi-line SQL when replaying a search (#3382)
## Summary Multi-line SQL in a dashboard's "Replay search" or drill-down link lost its line breaks as soon as the search page updated its URL, for example on a time-range change. A query like ServiceName = 'x' and + newline + SeverityText = 'error' became ...andSeverityText... and failed with a ClickHouse syntax error. Cause: buildEventsSearchUrl built the /search URL with a plain URLSearchParams, which encodes values once. The search page's where, select and filters parsers (parseAsStringEncoded / parseAsJsonEncoded) expect two encoding layers. With only one, the first URL write on the search page re-rendered where with a raw newline. nuqs doesn't escape newlines, so the browser stripped it, and nuqs then re-read the joined value into state. The same missing layer also corrupted literal %XX text in values (a%20b was read as a b). Fix: buildEventsSearchUrl now encodes where, select and filters with the same serializer the parsers use. It lives in a new nuqs-free module, utils/urlParamEncoding.ts, which queryParsers.ts now uses too. This fixes the four dashboard and chart links that use this builder: - tile Replay search - the time chart View All Events drill-down - table row click-through - the heatmap event-deltas link Old single-encoded links still parse. Other /search link builders (generateSearchUrl, copyRowUrl, service map, LLM dashboard) have the same problem and will be handled in a follow-up. ## How to reproduce Starting point: play.hyperdx.io On main: 1. Create a dashboard and add a line tile on a logs source. 2. Set the where language to SQL and enter ServiceName != '' AND, then press Shift+Enter, then SeverityText != ''. The line break must be the only whitespace between AND and SeverityText. Save the tile. 3. Hover the tile and click Replay search. Results load. 4. Change the time range, e.g. to "Last 1 days". 5. The where box shows a single line (...ANDSeverityText...) and the search fails with a syntax error. On this branch: 1. Repeat steps 1–4. The where box keeps both lines and results load. ## How to test on Vercel preview Preview routes: /dashboards Steps: 1. Create a new dashboard and add a line tile on the logs source. 2. Set the where language to SQL and enter ServiceName != '' AND, press Shift+Enter, then enter SeverityText != ''. Save the tile. 3. Hover the tile and click Replay search. In the new tab, wait for results. 4. Open the time picker and select "Last 1 days". 5. Verify the where editor shows two lines and the results table has rows, with no error. References - Linear Issue: Closes HDX-5726 - Related PRs:
1 parent 8b92b7a commit e3f24b9

9 files changed

Lines changed: 230 additions & 30 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@hyperdx/app': patch
3+
---
4+
5+
fix: Preserve multi-line SQL in search replay links

‎packages/app/src/ChartUtils.tsx‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ import { notifications } from '@mantine/notifications';
5151

5252
import DateRangeIndicator from './components/charts/DateRangeIndicator';
5353
import { MVOptimizationExplanationResult } from './hooks/useMVOptimizationExplanation';
54+
import {
55+
encodeJsonUrlParamValue,
56+
encodeUrlParamValue,
57+
} from './utils/urlParamEncoding';
5458
import {
5559
DEFAULT_SERIES_LIMIT,
5660
MAX_RENDERED_TIME_CHART_SERIES,
@@ -1380,11 +1384,19 @@ export function buildEventsSearchUrl({
13801384
const from = dateRange[0].getTime();
13811385
const to = dateRange[1].getTime();
13821386

1387+
// `where`, `select` and `filters` carry the search page parsers' own encoding
1388+
// layer under the query string's. When the search page later writes any
1389+
// other param, nuqs re-renders every value from the once-decoded query string
1390+
// without escaping newlines, and the browser strips them — so a
1391+
// single-encoded multi-line `where` comes back with its lines joined.
13831392
const params: Record<string, string> = {
13841393
source: source?.id ?? '',
1385-
where: where,
1394+
where: encodeUrlParamValue(where),
13861395
whereLanguage: whereLanguage,
1387-
filters: JSON.stringify([...(config.filters ?? []), ...additionalFilters]),
1396+
filters: encodeJsonUrlParamValue([
1397+
...(config.filters ?? []),
1398+
...additionalFilters,
1399+
]),
13881400
isLive: 'false',
13891401
from: from.toString(),
13901402
to: to.toString(),
@@ -1394,7 +1406,7 @@ export function buildEventsSearchUrl({
13941406
if (isMetricChart) {
13951407
params.where = '';
13961408
params.whereLanguage = 'lucene';
1397-
params.filters = JSON.stringify([]);
1409+
params.filters = encodeJsonUrlParamValue([]);
13981410
params.source =
13991411
(source.kind === SourceKind.Metric || source.kind === SourceKind.Trace
14001412
? source.logSourceId
@@ -1405,7 +1417,7 @@ export function buildEventsSearchUrl({
14051417
// eventTableSelect is used for charts that override select (like histograms with count)
14061418
// to preserve the original table's select expression
14071419
if (config.eventTableSelect) {
1408-
params.select = config.eventTableSelect;
1420+
params.select = encodeUrlParamValue(config.eventTableSelect);
14091421
}
14101422

14111423
return `/search?${new URLSearchParams(params).toString()}`;

‎packages/app/src/__tests__/dashboardReplaySearch.test.ts‎

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ import {
99

1010
import { buildDashboardReplaySearchUrl } from '@/ChartUtils';
1111

12+
// The search page reads these params with nuqs (double-encoded). Mirror that
13+
// read path so the tests assert the value the destination actually recovers.
14+
const readWhere = (url: URL) =>
15+
decodeURIComponent(url.searchParams.get('where') ?? '');
16+
const readFilters = (url: URL) =>
17+
JSON.parse(decodeURIComponent(url.searchParams.get('filters') ?? '[]'));
18+
1219
const dateRange: [Date, Date] = [
1320
new Date('2026-01-01T00:00:00.000Z'),
1421
new Date('2026-01-01T01:00:00.000Z'),
@@ -66,8 +73,8 @@ describe('buildDashboardReplaySearchUrl', () => {
6673
const url = new URL(result ?? '', 'http://localhost');
6774
expect(url.pathname).toBe('/search');
6875
expect(url.searchParams.get('source')).toBe('logs');
69-
expect(url.searchParams.get('where')).toBe('service:web');
70-
expect(JSON.parse(url.searchParams.get('filters') ?? '')).toEqual([
76+
expect(readWhere(url)).toBe('service:web');
77+
expect(readFilters(url)).toEqual([
7178
{ type: 'sql', condition: "SeverityText = 'error'" },
7279
]);
7380
expect(url.searchParams.get('from')).toBe(
@@ -93,9 +100,47 @@ describe('buildDashboardReplaySearchUrl', () => {
93100
});
94101

95102
expect(result).not.toBeNull();
96-
expect(
97-
new URL(result ?? '', 'http://localhost').searchParams.get('where'),
98-
).toBe('status:error');
103+
expect(readWhere(new URL(result ?? '', 'http://localhost'))).toBe(
104+
'status:error',
105+
);
106+
});
107+
108+
it('double-encodes multi-line where, filters, and select', () => {
109+
const multiline = (...lines: string[]) => lines.join('\n');
110+
const where = multiline(
111+
"ServiceName = 'receive' and",
112+
"ResourceAttributes['cluster_type'] = 'data-plane' and\tNamespace = 'x'",
113+
);
114+
const filters = [
115+
{ type: 'sql' as const, condition: multiline('a = 1 and', 'b = 2\r') },
116+
];
117+
const select = multiline('Timestamp,', 'Body');
118+
119+
const result = buildDashboardReplaySearchUrl({
120+
source: logSource,
121+
config: {
122+
...builderConfig,
123+
where,
124+
whereLanguage: 'sql',
125+
filters,
126+
eventTableSelect: select,
127+
},
128+
dateRange,
129+
});
130+
expect(result).not.toBeNull();
131+
132+
// When the search page writes any other param, nuqs re-renders these from
133+
// the once-decoded query string without escaping control characters, and
134+
// the browser strips them. One decode must leave none behind.
135+
const url = new URL(result ?? '', 'http://localhost');
136+
for (const key of ['where', 'filters', 'select']) {
137+
expect(url.searchParams.get(key)).not.toMatch(/[\n\r\t]/);
138+
}
139+
expect(readWhere(url)).toBe(where);
140+
expect(readFilters(url)).toEqual(filters);
141+
expect(decodeURIComponent(url.searchParams.get('select') ?? '')).toBe(
142+
select,
143+
);
99144
});
100145

101146
it('rejects per-series conditions that cannot be faithfully replayed', () => {

‎packages/app/src/__tests__/eventsSearchUrl.test.ts‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,18 @@ const builderConfig = {
4646
dateRange,
4747
} satisfies BuilderChartConfigWithDateRange;
4848

49+
// The search page reads these params with nuqs, which double-encodes values.
50+
// Mirror that read (`URLSearchParams.get` decodes one layer, the parser's
51+
// `decodeURIComponent` the second) so assertions compare the recovered value.
4952
const searchParams = (url: string | null) => {
5053
expect(url).not.toBeNull();
51-
return new URL(url ?? '', 'http://localhost').searchParams;
54+
const raw = new URL(url ?? '', 'http://localhost').searchParams;
55+
return {
56+
get: (key: string) => {
57+
const value = raw.get(key);
58+
return value == null ? null : decodeURIComponent(value);
59+
},
60+
};
5261
};
5362

5463
const buildUrl = (
@@ -171,11 +180,16 @@ describe('buildEventsSearchUrl variable expansion', () => {
171180
});
172181

173182
it('leaves a config without variables untouched', () => {
174-
expect(
183+
const params = searchParams(
175184
buildUrl({ where: 'ServiceName = $svc', whereLanguage: 'sql' }),
176-
).toBe(
177-
'/search?source=logs&where=ServiceName+%3D+%24svc&whereLanguage=sql&filters=%5B%5D&isLive=false&from=1767225600000&to=1767229200000',
178185
);
186+
expect(params.get('source')).toBe('logs');
187+
expect(params.get('where')).toBe('ServiceName = $svc');
188+
expect(params.get('whereLanguage')).toBe('sql');
189+
expect(params.get('filters')).toBe('[]');
190+
expect(params.get('isLive')).toBe('false');
191+
expect(params.get('from')).toBe('1767225600000');
192+
expect(params.get('to')).toBe('1767229200000');
179193
});
180194

181195
it('falls back to the clause as written when expansion fails', () => {

‎packages/app/src/utils/queryParsers.ts‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ import {
55
} from '@hyperdx/common-utils/dist/types';
66
import { SortingState } from '@tanstack/react-table';
77

8+
import {
9+
decodeUrlParamValue,
10+
encodeJsonUrlParamValue,
11+
encodeUrlParamValue,
12+
} from './urlParamEncoding';
13+
814
/**
915
* Problem: nuqs serializes spaces as '+' (form-encoding). When
1016
* URLs are shared via Microsoft Teams (and some other systems), they re-encode
@@ -25,15 +31,8 @@ import { SortingState } from '@tanstack/react-table';
2531
* as %0A automatically).
2632
*/
2733
export const parseAsStringEncoded = createParser<string>({
28-
parse: value => {
29-
try {
30-
return decodeURIComponent(value);
31-
} catch {
32-
// Malformed URI sequence – return as-is for robustness.
33-
return value;
34-
}
35-
},
36-
serialize: value => encodeURIComponent(value),
34+
parse: decodeUrlParamValue,
35+
serialize: encodeUrlParamValue,
3736
});
3837

3938
/**
@@ -80,12 +79,12 @@ export function parseAsJsonEncoded<T>(
8079
}
8180

8281
try {
83-
return finalize(JSON.parse(decodeURIComponent(value)));
82+
return finalize(JSON.parse(decodeUrlParamValue(value)));
8483
} catch {
8584
return null;
8685
}
8786
},
88-
serialize: value => encodeURIComponent(JSON.stringify(value)),
87+
serialize: encodeJsonUrlParamValue,
8988
});
9089
}
9190

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* The inner encoding layer of `parseAsStringEncoded` / `parseAsJsonEncoded`
3+
* (see `queryParsers.ts` for why it exists). Kept free of nuqs so code that
4+
* builds links to those params can share it without loading nuqs at import
5+
* time.
6+
*/
7+
export const encodeUrlParamValue = (value: string) => encodeURIComponent(value);
8+
9+
export const decodeUrlParamValue = (value: string) => {
10+
try {
11+
return decodeURIComponent(value);
12+
} catch {
13+
// Malformed URI sequence – return as-is for robustness.
14+
return value;
15+
}
16+
};
17+
18+
export const encodeJsonUrlParamValue = (value: unknown) =>
19+
encodeUrlParamValue(JSON.stringify(value));

‎packages/app/tests/e2e/components/ChartEditorComponent.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,24 @@ export class ChartEditorComponent {
209209
await dismissSqlAutocomplete(this.page);
210210
}
211211

212+
/**
213+
* Type a multi-line SQL WHERE clause, one line per entry. Lines are split
214+
* with Shift+Enter, since a bare Enter in the editor submits the query.
215+
*/
216+
async setMultilineSqlWhere(
217+
lines: string[],
218+
scope: 'chart' | 'series' = 'chart',
219+
) {
220+
const [first = '', ...rest] = lines;
221+
await this.setWhereLanguage('SQL', scope);
222+
await this.whereInput(scope).fillSql(first);
223+
for (const line of rest) {
224+
await this.page.keyboard.press('Shift+Enter');
225+
await this.page.keyboard.type(line);
226+
}
227+
await dismissSqlAutocomplete(this.page);
228+
}
229+
212230
/**
213231
* Type into a WHERE input while it is in Lucene mode, where it renders as a
214232
* plain textarea rather than CodeMirror. Leaves the suggestion dropdown open.

‎packages/app/tests/e2e/features/dashboard-table-linking.spec.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,11 @@ test.describe(
206206
expect(url.searchParams.get('source')).toBe(logsSourceId);
207207
expect(url.searchParams.get('isLive')).toBe('false');
208208
// Default row click encodes the group-by filter as a SQL IN clause in
209-
// the JSON-encoded `filters` param.
210-
const filters = url.searchParams.get('filters') ?? '[]';
209+
// the JSON-encoded `filters` param. The search page reads it with nuqs
210+
// (double-encoded), so decode the extra layer before asserting.
211+
const filters = decodeURIComponent(
212+
url.searchParams.get('filters') ?? '[]',
213+
);
211214
expect(filters).toContain(`ServiceName IN ('${serviceName}')`);
212215
const from = Number(url.searchParams.get('from'));
213216
const to = Number(url.searchParams.get('to'));

‎packages/app/tests/e2e/features/dashboard.spec.ts‎

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,85 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
14221422
},
14231423
);
14241424

1425+
test(
1426+
'replay search keeps a multi-line SQL where after the search page rewrites the URL',
1427+
{ tag: '@full-stack' },
1428+
async () => {
1429+
const chartName = `E2E Multiline Replay Tile ${Date.now()}`;
1430+
// The line break must be the only whitespace between the two tokens: the
1431+
// browser strips raw newlines from URLs but keeps spaces, so an indented
1432+
// second line would survive the bug as valid SQL.
1433+
const whereLines = ["ServiceName != '' AND", "SeverityText != ''"];
1434+
const multilineWhere = whereLines.join('\n');
1435+
1436+
await test.step('Save a builder tile with a multi-line SQL where', async () => {
1437+
await dashboardPage.createNewDashboard();
1438+
await dashboardPage.addTile();
1439+
await expect(dashboardPage.chartEditor.nameInput).toBeVisible();
1440+
await dashboardPage.chartEditor.waitForDataToLoad();
1441+
await dashboardPage.chartEditor.setChartType(DisplayType.Line);
1442+
await dashboardPage.chartEditor.setChartName(chartName);
1443+
await dashboardPage.chartEditor.selectSource(DEFAULT_LOGS_SOURCE_NAME);
1444+
await dashboardPage.chartEditor.setMultilineSqlWhere(whereLines);
1445+
await dashboardPage.chartEditor.runQuery();
1446+
await dashboardPage.saveTile();
1447+
});
1448+
1449+
const tile = dashboardPage.getTiles().filter({ hasText: chartName });
1450+
const replayButton = tile.locator(
1451+
'[data-testid^="tile-replay-search-button-"]',
1452+
);
1453+
1454+
await test.step('The replay link carries the where with its line break', async () => {
1455+
await tile.hover();
1456+
await expect(replayButton).toBeVisible();
1457+
const href = String(await replayButton.getAttribute('href'));
1458+
const where = String(
1459+
new URL(href, 'http://localhost').searchParams.get('where'),
1460+
);
1461+
expect(decodeURIComponent(where)).toBe(multilineWhere);
1462+
});
1463+
1464+
await test.step('Changing the time range on the search page keeps the line break', async () => {
1465+
const popupPromise = dashboardPage.page.waitForEvent('popup');
1466+
await replayButton.click();
1467+
const searchTab = await popupPromise;
1468+
const searchPage = new SearchPage(searchTab);
1469+
const readWhere = () =>
1470+
decodeURIComponent(
1471+
new URL(searchTab.url()).searchParams.get('where') ?? '',
1472+
);
1473+
1474+
await expect(searchTab).toHaveURL(/\/search\?/, { timeout: 15000 });
1475+
await expect(searchPage.table.firstRow).toBeVisible({
1476+
timeout: 30000,
1477+
});
1478+
const landedFrom = new URL(searchTab.url()).searchParams.get('from');
1479+
1480+
// A nuqs write that doesn't touch `where` rebuilds the whole query
1481+
// string from the decoded params. A single-encoded `where` comes back
1482+
// with a raw newline that the browser strips, joining the lines.
1483+
await searchPage.timePicker.selectRelativeTime('Last 1 days');
1484+
await expect
1485+
.poll(() => new URL(searchTab.url()).searchParams.get('from'), {
1486+
timeout: 10000,
1487+
})
1488+
.not.toBe(landedFrom);
1489+
1490+
expect(readWhere()).toBe(multilineWhere);
1491+
await expect(
1492+
searchPage.whereInput.sqlEditor.locator('.cm-line'),
1493+
).toHaveCount(2);
1494+
await expect(searchPage.table.firstRow).toBeVisible({
1495+
timeout: 30000,
1496+
});
1497+
await expect(searchPage.chartErrorState).toHaveCount(0);
1498+
1499+
await searchTab.close();
1500+
});
1501+
},
1502+
);
1503+
14251504
test.describe('Raw SQL Dashboard Tiles', () => {
14261505
const LINE_SQL = `SELECT toStartOfInterval(Timestamp, INTERVAL {intervalSeconds:Int64} SECOND) AS ts, count() AS count FROM default.e2e_otel_logs WHERE Timestamp >= fromUnixTimestamp64Milli({startDateMilliseconds:Int64}) AND Timestamp < fromUnixTimestamp64Milli({endDateMilliseconds:Int64}) GROUP BY ts ORDER BY ts ASC`;
14271506

@@ -2289,7 +2368,11 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
22892368

22902369
const href = (await link.getAttribute('href')) ?? '';
22912370
const params = new URL(href, 'http://localhost').searchParams;
2292-
expect(params.get('where')).toBe("(ServiceName IN ('accounting'))");
2371+
// The search page reads `where` with nuqs (double-encoded), so decode
2372+
// the extra layer before asserting on the expanded predicate.
2373+
expect(decodeURIComponent(params.get('where') ?? '')).toBe(
2374+
"(ServiceName IN ('accounting'))",
2375+
);
22932376
expect(decodeURIComponent(href)).not.toContain('$svc');
22942377
});
22952378

@@ -2304,9 +2387,11 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
23042387
const searchPage = new SearchPage(searchTab);
23052388

23062389
await expect(searchTab).toHaveURL(/\/search\?/, { timeout: 15000 });
2307-
expect(new URL(searchTab.url()).searchParams.get('where')).toBe(
2308-
"(ServiceName IN ('accounting'))",
2309-
);
2390+
expect(
2391+
decodeURIComponent(
2392+
new URL(searchTab.url()).searchParams.get('where') ?? '',
2393+
),
2394+
).toBe("(ServiceName IN ('accounting'))");
23102395

23112396
await expect(searchPage.table.firstRow).toBeVisible({
23122397
timeout: 30000,

0 commit comments

Comments
 (0)