Skip to content

fix(knative): clean up blob URL and DOM node after log download - #1223

Open
magic-peach wants to merge 1 commit into
headlamp-k8s:mainfrom
magic-peach:fix/knative-log-download-leak
Open

fix(knative): clean up blob URL and DOM node after log download#1223
magic-peach wants to merge 1 commit into
headlamp-k8s:mainfrom
magic-peach:fix/knative-log-download-leak

Conversation

@magic-peach

Copy link
Copy Markdown

Downloading logs from the KService log viewer creates a Blob URL and an anchor element for every click, but never revokes the URL or removes the element afterward. Each download leaves a detached <a> in the DOM permanently and pins the log Blob in memory for the rest of the session — noticeable on a page where someone re-downloads logs repeatedly while debugging.

Both kompose's download button and ai-assistant's LogsDialog already do this correctly in this same repo (the latter uses a try/finally so cleanup happens even if click() throws), so I matched that pattern here instead of inventing a new one.

Verified the before/after behavior with a standalone script simulating both code paths — the old one never calls remove()/revokeObjectURL, the new one calls both every time regardless of whether click() succeeds. Didn't add a full component-render test since exercising this function means standing up the whole KService pods/activity/permissions mocking chain for a five-line cleanup fix, which felt disproportionate — happy to add one if a maintainer wants it.

downloadPlainLogs creates an anchor element and an object URL for every
download but never removes either. kompose's download button and
ai-assistant's LogsDialog both already revoke the URL and remove the
element right after triggering the click (LogsDialog does it in a
finally so it still happens if click() throws) - this one didn't, so
each download left a detached anchor in the DOM and pinned the blob in
memory for the rest of the session. Same pattern as those two now.

Signed-off-by: Akanksha Trehun <akankshatrehun@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant