mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-05 16:10:43 +00:00
fix(logs): stop lazy-editor's transient empty emission wiping restored share-link query (#13426)
## The bug
Opening a **shared link** to an SQL search intermittently lands on an
**empty query editor** ("SQL query is missing or invalid"). Recurring
Logs-Features alpha flake: `monaco-query-prefill` P0 / `shareLink` P1 →
*"query editor never contained SELECT."*
Follow-up to #13414 — that PR guarded a plausible path (the
`fullSQLMode` watcher's "switch ON"), but the trace after it deployed
showed the query **still** dropped. This finds and fixes the actual
cause.
## Root cause (trace-confirmed)
The Monaco query editor is **lazy-loaded**. After the short-URL restore
sets the SQL query, the editor fires its change callback with an empty
`""` **while it mounts — before the restored value is applied.**
`SearchBar.updateQueryValue()` treated that transient empty as a real
edit:
```js
searchObj.data.editorValue = value; // "" overwrites the restored query
searchObj.data.query = value;
if (value.trim() === "" && searchObj.meta.sqlMode === true) {
searchObj.meta.sqlMode = false; // → SQL mode auto-detects OFF → editor cleared for good
}
```
Under load the editor's empty emission lands *after* the restore, so the
query is lost. The trace shows the URL ending `sql_mode=false` with the
`query` param gone — exactly this.
## The fix — state-based, not a timer
The editor emits `""` *because it isn't ready yet*. So we gate on
**readiness (state)**, not a wall-clock wait (a fixed delay would just
change the odds, like "make the env faster"):
- `restoreUrlQueryParams()` sets `meta.pendingUrlQueryRestore = true`
when it restores a non-empty query.
- `updateQueryValue()` **ignores an empty emission while that flag is
set**, and clears the flag the instant the real (non-empty) value lands
— so genuine user clears *after* load still work. This mirrors the
existing `loadingStream` guard already in that function.
Because it keys off state, the race can't beat it: no matter *when* the
empty emission arrives, it's ignored until the real value is in.
## Verification
- ✅ **New real unit test** on `restoreUrlQueryParams`
(`useLogs.spec.ts`, actual composable) — asserts the base64 query
decodes into state, SQL mode restores, and `pendingUrlQueryRestore` is
set. **7 passed.**
- ✅ The guard is deterministic by construction (reads the same shared
`searchObj.meta`).
- ⚠️ The load-race is only observable against a **deployed** frontend,
so the final "flake gone on alpha under load" confirmation is a re-run
once this is deployed to alpha. (I was wrong once here — so I'm being
explicit that this last step is pending, not assumed.)
## Note
#13414's guard is left in place — it's a valid defensive symmetry fix,
just not the operative cause. This PR is the operative fix.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
df9d35c3d3
commit
c122fbf9dc
@@ -533,6 +533,28 @@ describe("Use Logs Composable", () => {
|
||||
expect(wrapper.vm.searchObj.data.datetime.relativeTimePeriod).toBe("15m");
|
||||
});
|
||||
|
||||
it("should decode the SQL query and flag pendingUrlQueryRestore (share-link restore guard)", async () => {
|
||||
// base64 of: SELECT * FROM "e2e_automate" (matches the real short-URL `query` param)
|
||||
wrapper.vm.router.currentRoute.value.query = {
|
||||
stream: "e2e_automate",
|
||||
period: "15m",
|
||||
sql_mode: "true",
|
||||
query: "U0VMRUNUICogRlJPTSAiZTJlX2F1dG9tYXRlIg==",
|
||||
};
|
||||
|
||||
const { restoreUrlQueryParams } = wrapper.vm;
|
||||
await restoreUrlQueryParams();
|
||||
await nextTick();
|
||||
|
||||
// Query is decoded into state and SQL mode restored
|
||||
expect(wrapper.vm.searchObj.data.query).toBe('SELECT * FROM "e2e_automate"');
|
||||
expect(wrapper.vm.searchObj.data.editorValue).toBe('SELECT * FROM "e2e_automate"');
|
||||
expect(wrapper.vm.searchObj.meta.sqlMode).toBe(true);
|
||||
// Restore is flagged pending so the lazy editor's transient empty emission
|
||||
// can't wipe the restored query in updateQueryValue().
|
||||
expect(wrapper.vm.searchObj.meta.pendingUrlQueryRestore).toBe(true);
|
||||
});
|
||||
|
||||
it("should handle SQL mode and encoded query", async () => {
|
||||
const encodedQuery = btoa("SELECT * FROM logs");
|
||||
wrapper.vm.router.currentRoute.value.query = {
|
||||
|
||||
@@ -394,6 +394,15 @@ const useLogs = () => {
|
||||
const decodedQuery = b64DecodeUnicode(queryParams.query)!;
|
||||
searchObj.data.editorValue = decodedQuery;
|
||||
searchObj.data.query = decodedQuery;
|
||||
// The Monaco query editor is lazy-loaded and, while it mounts, fires its
|
||||
// change callback with an empty "" BEFORE this restored value is applied.
|
||||
// Flag the restore as pending so updateQueryValue() ignores that transient
|
||||
// empty emission instead of wiping the query (and flipping SQL mode off) —
|
||||
// the intermittent "shared SQL link opens an empty editor" bug. The flag is
|
||||
// cleared the moment the real (non-empty) value lands in the editor.
|
||||
if (decodedQuery.trim() !== "") {
|
||||
searchObj.meta.pendingUrlQueryRestore = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (
|
||||
|
||||
@@ -2876,6 +2876,20 @@ export default defineComponent({
|
||||
return;
|
||||
}
|
||||
|
||||
// A URL / short-link restore has just set the SQL query, but the lazy-loaded
|
||||
// Monaco editor fires this callback with an empty "" as it mounts, BEFORE the
|
||||
// restored value is applied. Treating that transient empty as a real edit
|
||||
// wipes searchObj.data.query and (below) flips SQL mode off — the intermittent
|
||||
// "shared SQL link opens an empty editor" bug. While a restore is pending,
|
||||
// ignore the empty emission; clear the flag as soon as the real (non-empty)
|
||||
// value lands so genuine later clears by the user still work.
|
||||
if (searchObj.meta.pendingUrlQueryRestore) {
|
||||
if (value.trim() === "") {
|
||||
return;
|
||||
}
|
||||
searchObj.meta.pendingUrlQueryRestore = false;
|
||||
}
|
||||
|
||||
// if (searchObj.meta.jobId != "") {
|
||||
// searchObj.meta.jobId = "";
|
||||
// getQueryData(false);
|
||||
|
||||
@@ -123,6 +123,11 @@ export const DEFAULT_LOGS_CONFIG = {
|
||||
sqlMode: false,
|
||||
sqlModeManualTrigger: false,
|
||||
sqlModeEditTransition: false,
|
||||
// True from when a URL / short-link restore sets the query until the
|
||||
// (lazy-loaded) editor has actually applied it. Guards updateQueryValue()
|
||||
// against the editor's transient empty emission during mount wiping the
|
||||
// restored query. See SearchBar.updateQueryValue and useLogs.restoreUrlQueryParams.
|
||||
pendingUrlQueryRestore: false,
|
||||
nlpMode: false,
|
||||
quickMode: false,
|
||||
// True when the current selectedFields were chosen automatically by the FTS
|
||||
|
||||
Reference in New Issue
Block a user