From c122fbf9dcd2d81bc2587be1e44950454bfb8dd6 Mon Sep 17 00:00:00 2001 From: Shrinath Rao Date: Fri, 24 Jul 2026 09:56:46 +0530 Subject: [PATCH] fix(logs): stop lazy-editor's transient empty emission wiping restored share-link query (#13426) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- web/src/composables/useLogs.spec.ts | 22 ++++++++++++++++++++++ web/src/composables/useLogs.ts | 9 +++++++++ web/src/plugins/logs/SearchBar.vue | 14 ++++++++++++++ web/src/utils/logs/constants.ts | 5 +++++ 4 files changed, 50 insertions(+) diff --git a/web/src/composables/useLogs.spec.ts b/web/src/composables/useLogs.spec.ts index aea9f58411..3d59325a52 100644 --- a/web/src/composables/useLogs.spec.ts +++ b/web/src/composables/useLogs.spec.ts @@ -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 = { diff --git a/web/src/composables/useLogs.ts b/web/src/composables/useLogs.ts index 9815a2cbac..abaab85aff 100644 --- a/web/src/composables/useLogs.ts +++ b/web/src/composables/useLogs.ts @@ -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 ( diff --git a/web/src/plugins/logs/SearchBar.vue b/web/src/plugins/logs/SearchBar.vue index 056bac868f..802e7f2908 100644 --- a/web/src/plugins/logs/SearchBar.vue +++ b/web/src/plugins/logs/SearchBar.vue @@ -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); diff --git a/web/src/utils/logs/constants.ts b/web/src/utils/logs/constants.ts index da83665ad0..cd52c99d57 100644 --- a/web/src/utils/logs/constants.ts +++ b/web/src/utils/logs/constants.ts @@ -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