github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-04-25 11:18:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix removeChild errors from unmount race conditions (#8927)

Browse Source 
Guard imperative DOM operations (removeChild, RAF callbacks) against
race conditions where React has already unmounted the target nodes.

- assets/common.cljs: add dom/child? guard before removeChild in RAF
- dynamic_modifiers.cljs: capture RAF IDs and cancel them on cleanup;
  add null guards for DOM nodes that may no longer exist
- hooks.cljs: guard portal container removal with dom/child? check
- errors.cljs: extract is-ignorable-exception? to a top-level defn
  and add NotFoundError/removeChild to ignorable exceptions, since
  these are caused by browser extensions modifying React-managed DOM
- Add unit tests for is-ignorable-exception? predicate

Signed-off-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
Andrey Antukh 2026-04-21 17:31:05 +02:00 committed by GitHub
parent d5cf7dcf9d
commit aed2f8a8f8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 206 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
frontend/src/app/main/errors.cljs
View File

@ -408,43 +408,61 @@
(ex/print-throwable instance :prefix "Server Error"))
(st/async-emit! (rt/assign-exception error)))
(defn- from-extension?
"True when the error stack trace originates from a browser extension."
[cause]
(let [stack (.-stack cause)]
(and (string? stack)
(or (str/includes? stack "chrome-extension://")
(str/includes? stack "moz-extension://")))))
(defn- from-posthog?
"True when the error stack trace originates from PostHog analytics."
[cause]
(let [stack (.-stack cause)]
(and (string? stack)
(str/includes? stack "posthog"))))
(defn is-ignorable-exception?
"True when the error is known to be harmless (browser extensions, analytics,
React/extension DOM conflicts, etc.) and should NOT be surfaced to the user."
[cause]
(let [message (ex-message cause)]
(or (from-extension? cause)
(from-posthog? cause)
(= message "Possible side-effect in debug-evaluate")
(= message "Unexpected end of input")
(str/starts-with? message "invalid props on component")
(str/starts-with? message "Unexpected token ")
;; Native AbortError DOMException: raised when an in-flight
;; HTTP fetch is cancelled via AbortController (e.g. by an
;; RxJS unsubscription / take-until chain). These are
;; handled gracefully inside app.util.http/fetch and must NOT
;; be surfaced as application errors.
(= (.-name ^js cause) "AbortError")
;; Zone.js (injected by browser extensions such as Angular
;; DevTools) wraps event listeners and assigns a custom
;; .toString to its wrapper functions using
;; Object.defineProperty. When the wrapper was previously
;; defined with {writable: false}, a subsequent plain assignment
;; in strict mode (our libs.js uses "use strict") throws this
;; TypeError. This is a known Zone.js / browser-extension
;; incompatibility and is NOT a Penpot bug.
(str/starts-with? message "Cannot assign to read only property 'toString'")
;; NotFoundError DOMException: "Failed to execute
;; 'removeChild' on 'Node'" — Thrown by React's commit
;; phase when the DOM tree has been modified externally
;; (typically by browser extensions like Grammarly,
;; LastPass, translation tools, or ad blockers that
;; inject/remove nodes). The entire stack trace is inside
;; React internals (libs.js) with no application code,
;; so there is nothing actionable on our side. React's
;; error boundary already handles recovery.
(and (= (.-name ^js cause) "NotFoundError")
(str/includes? message "removeChild")))))
(defonce uncaught-error-handler
(letfn [(from-extension? [cause]
(let [stack (.-stack cause)]
(and (string? stack)
(or (str/includes? stack "chrome-extension://")
(str/includes? stack "moz-extension://")))))
(from-posthog? [cause]
(let [stack (.-stack cause)]
(and (string? stack)
(str/includes? stack "posthog"))))
(is-ignorable-exception? [cause]
(let [message (ex-message cause)]
(or (from-extension? cause)
(from-posthog? cause)
(= message "Possible side-effect in debug-evaluate")
(= message "Unexpected end of input")
(str/starts-with? message "invalid props on component")
(str/starts-with? message "Unexpected token ")
;; Native AbortError DOMException: raised when an in-flight
;; HTTP fetch is cancelled via AbortController (e.g. by an
;; RxJS unsubscription / take-until chain). These are
;; handled gracefully inside app.util.http/fetch and must NOT
;; be surfaced as application errors.
(= (.-name ^js cause) "AbortError")
;; Zone.js (injected by browser extensions such as Angular
;; DevTools) wraps event listeners and assigns a custom
;; .toString to its wrapper functions using
;; Object.defineProperty. When the wrapper was previously
;; defined with {writable: false}, a subsequent plain assignment
;; in strict mode (our libs.js uses "use strict") throws this
;; TypeError. This is a known Zone.js / browser-extension
;; incompatibility and is NOT a Penpot bug.
(str/starts-with? message "Cannot assign to read only property 'toString'"))))
(on-unhandled-error [event]
(letfn [(on-unhandled-error [event]
(.preventDefault ^js event)
(when-let [cause (unchecked-get event "error")]
(when-not (is-ignorable-exception? cause)

86
frontend/src/app/main/ui/workspace/shapes/frame/dynamic_modifiers.cljs
View File

@ -265,54 +265,68 @@
prev-transforms (mf/use-var nil)]
(mf/with-effect [add-children]
(ts/raf
#(doseq [{:keys [shape]} add-children-prev]
(let [shape-node (get-shape-node shape)
mirror-node (dom/query (dm/fmt ".mirror-shape[href='#shape-%'" shape))]
(when mirror-node (.remove mirror-node))
(dom/remove-attribute! (dom/get-parent shape-node) "display"))))
(let [raf-id1
(ts/raf
#(doseq [{:keys [shape]} add-children-prev]
(let [shape-node (get-shape-node shape)
mirror-node (dom/query (dm/fmt ".mirror-shape[href='#shape-%'" shape))]
(when mirror-node (.remove mirror-node))
(when-let [parent (some-> shape-node dom/get-parent)]
(dom/remove-attribute! parent "display")))))
(ts/raf
#(doseq [{:keys [frame shape]} add-children]
(let [frame-node (get-shape-node frame)
shape-node (get-shape-node shape)
raf-id2
(ts/raf
#(doseq [{:keys [frame shape]} add-children]
(let [frame-node (get-shape-node frame)
shape-node (get-shape-node shape)]
(when (and (some? frame-node) (some? shape-node))
(let [clip-id
(-> (dom/query frame-node ":scope > defs > .frame-clip-def")
(dom/get-attribute "id"))
clip-id
(-> (dom/query frame-node ":scope > defs > .frame-clip-def")
(dom/get-attribute "id"))
use-node
(dom/create-element "http://www.w3.org/2000/svg" "use")
use-node
(dom/create-element "http://www.w3.org/2000/svg" "use")
contents-node
(or (dom/query frame-node ".frame-children") frame-node)]
contents-node
(or (dom/query frame-node ".frame-children") frame-node)]
(dom/set-attribute! use-node "href" (dm/fmt "#shape-%" shape))
(dom/set-attribute! use-node "clip-path" (dm/fmt "url(#%)" clip-id))
(dom/add-class! use-node "mirror-shape")
(dom/append-child! contents-node use-node)
(dom/set-attribute! (dom/get-parent shape-node) "display" "none")))))
(dom/set-attribute! use-node "href" (dm/fmt "#shape-%" shape))
(dom/set-attribute! use-node "clip-path" (dm/fmt "url(#%)" clip-id))
(dom/add-class! use-node "mirror-shape")
(dom/append-child! contents-node use-node)
(dom/set-attribute! (dom/get-parent shape-node) "display" "none"))))))]
(fn []
(js/cancelAnimationFrame raf-id1)
(js/cancelAnimationFrame raf-id2))))
(mf/with-effect [transforms]
(let [curr-shapes-set (into #{} (map :id) shapes)
prev-shapes-set (into #{} (map :id) @prev-shapes)
new-shapes (->> shapes (remove #(contains? prev-shapes-set (:id %))))
removed-shapes (->> @prev-shapes (remove #(contains? curr-shapes-set (:id %))))]
removed-shapes (->> @prev-shapes (remove #(contains? curr-shapes-set (:id %))))
;; NOTE: we schedule the dom modifications to be executed
;; asynchronously for avoid component flickering when react18
;; is used.
;; NOTE: we schedule the dom modifications to be executed
;; asynchronously for avoid component flickering when react18
;; is used.
(when (d/not-empty? new-shapes)
(ts/raf #(start-transform! node new-shapes)))
raf-id1
(when (d/not-empty? new-shapes)
(ts/raf #(start-transform! node new-shapes)))
(when (d/not-empty? shapes)
(ts/raf #(update-transform! node shapes transforms modifiers)))
raf-id2
(when (d/not-empty? shapes)
(ts/raf #(update-transform! node shapes transforms modifiers)))
(when (d/not-empty? removed-shapes)
(ts/raf #(remove-transform! node removed-shapes))))
raf-id3
(when (d/not-empty? removed-shapes)
(ts/raf #(remove-transform! node removed-shapes)))]
(reset! prev-modifiers modifiers)
(reset! prev-transforms transforms)
(reset! prev-shapes shapes))))
(reset! prev-modifiers modifiers)
(reset! prev-transforms transforms)
(reset! prev-shapes shapes)
(fn []
(when raf-id1 (js/cancelAnimationFrame raf-id1))
(when raf-id2 (js/cancelAnimationFrame raf-id2))
(when raf-id3 (js/cancelAnimationFrame raf-id3)))))))

6
frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs
View File

@ -242,7 +242,11 @@
;; afterwards, in the next render cycle.
(dom/append-child! item-el counter-el)
(dnd/set-drag-image! event item-el (:x offset) (:y offset))
(ts/raf #(.removeChild ^js item-el counter-el))))
;; Guard against race condition: if the user navigates away
;; before the RAF fires, item-el may have been unmounted and
;; counter-el is no longer a child — removeChild would throw.
(ts/raf #(when (dom/child? counter-el item-el)
(dom/remove-child! item-el counter-el)))))
(defn on-asset-drag-start
[event file-id asset selected item-ref asset-type on-drag-start]

95
frontend/test/frontend_tests/errors_test.cljs Normal file
View File

@ -0,0 +1,95 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS INC
(ns frontend-tests.errors-test
(:require
[app.main.errors :as errors]
[cljs.test :as t :include-macros true]))
(defn- make-error
"Create a JS Error-like object with the given name, message, and optional stack."
[error-name message & {:keys [stack] :or {stack ""}}]
(let [err (js/Error. message)]
(set! (.-name err) error-name)
(when (some? stack)
(set! (.-stack err) stack))
err))
;; ---------------------------------------------------------------------------
;; is-ignorable-exception? tests
;; ---------------------------------------------------------------------------
(t/deftest test-ignorable-chrome-extension
(t/testing "Errors from Chrome extensions are ignorable"
(let [cause (make-error "Error" "some error"
:stack "Error: some error\n at chrome-extension://abc123/content.js:1:1")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-ignorable-moz-extension
(t/testing "Errors from Firefox extensions are ignorable"
(let [cause (make-error "Error" "some error"
:stack "Error: some error\n at moz-extension://abc123/content.js:1:1")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-ignorable-posthog
(t/testing "Errors from PostHog are ignorable"
(let [cause (make-error "Error" "some error"
:stack "Error: some error\n at https://app.posthog.com/static/array.js:1:1")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-ignorable-debug-evaluate
(t/testing "Debug-evaluate side-effect errors are ignorable"
(let [cause (make-error "Error" "Possible side-effect in debug-evaluate")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-ignorable-unexpected-end-of-input
(t/testing "Unexpected end of input errors are ignorable"
(let [cause (make-error "SyntaxError" "Unexpected end of input")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-ignorable-invalid-props
(t/testing "Invalid React props errors are ignorable"
(let [cause (make-error "Error" "invalid props on component Foo")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-ignorable-unexpected-token
(t/testing "Unexpected token errors are ignorable"
(let [cause (make-error "SyntaxError" "Unexpected token <")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-ignorable-abort-error
(t/testing "AbortError DOMException is ignorable"
(let [cause (make-error "AbortError" "The operation was aborted")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-ignorable-zone-js-tostring
(t/testing "Zone.js toString read-only property error is ignorable"
(let [cause (make-error "TypeError"
"Cannot assign to read only property 'toString' of function 'function () { [native code] }'")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-ignorable-not-found-error-remove-child
(t/testing "NotFoundError with removeChild message is ignorable"
(let [cause (make-error "NotFoundError"
"Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node."
:stack "NotFoundError: Failed to execute 'removeChild'\n at zLe (libs.js:1:1)")]
(t/is (true? (errors/is-ignorable-exception? cause))))))
(t/deftest test-not-ignorable-not-found-error-other
(t/testing "NotFoundError without removeChild is NOT ignorable"
(let [cause (make-error "NotFoundError"
"Failed to execute 'insertBefore' on 'Node': something else")]
(t/is (false? (errors/is-ignorable-exception? cause))))))
(t/deftest test-not-ignorable-regular-error
(t/testing "Regular application errors are NOT ignorable"
(let [cause (make-error "Error" "Cannot read property 'x' of undefined")]
(t/is (false? (errors/is-ignorable-exception? cause))))))
(t/deftest test-not-ignorable-type-error
(t/testing "Regular TypeError is NOT ignorable"
(let [cause (make-error "TypeError" "undefined is not a function")]
(t/is (false? (errors/is-ignorable-exception? cause))))))

2
frontend/test/frontend_tests/runner.cljs
View File

@ -7,6 +7,7 @@
[frontend-tests.data.workspace-colors-test]
[frontend-tests.data.workspace-texts-test]
[frontend-tests.data.workspace-thumbnails-test]
[frontend-tests.errors-test]
[frontend-tests.helpers-shapes-test]
[frontend-tests.logic.comp-remove-swap-slots-test]
[frontend-tests.logic.components-and-tokens]
@ -42,6 +43,7 @@
(t/run-tests
'frontend-tests.basic-shapes-test
'frontend-tests.data.repo-test
'frontend-tests.errors-test
'frontend-tests.main-errors-test
'frontend-tests.data.viewer-test
'frontend-tests.data.workspace-colors-test