Skip to content

feat: add catch_unwind protection to render to prevent Node.js process crashes#396

Merged
yisibl merged 1 commit intomainfrom
catch_unwind
Jan 27, 2026
Merged

feat: add catch_unwind protection to render to prevent Node.js process crashes#396
yisibl merged 1 commit intomainfrom
catch_unwind

Conversation

@yisibl
Copy link
Copy Markdown
Member

@yisibl yisibl commented Jan 26, 2026

Background

When the upstream resvg (Rust) encounters an SVG that cannot be rendered, it may panic. This causes the Node.js process to crash immediately, resulting in a core dump.

✅ Node.js

There are also some upstream PRs working towards this goal, but they haven't been merged yet:

🚧 Wasm

By default the wasm32-unknown-unknown target is compiled with -Cpanic=abort. Historically this was due to the fact that there was no way to catch panics in wasm, but since mid-2025 the WebAssembly exception-handling proposal reached stabilization. LLVM has support for this proposal as well and when this is all combined together it's possible to enable -Cpanic=unwind on wasm targets.
See https://doc.rust-lang.org/rustc/platform-support/wasm32-unknown-unknown.html#unwinding

However, considering that this would increase the size of the wasm file and require runtime support, we will not enable it at this time.

@vercel
Copy link
Copy Markdown

vercel bot commented Jan 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
resvg-js Ready Ready Preview, Comment Jan 27, 2026 6:52am
@yisibl yisibl requested a review from Brooooooklyn January 26, 2026 13:10
@CGQAQ

This comment has been minimized.

…s crashes

- Use catch_unwind in the resvg-js Rust layer to capture panics, converting them into explicit errors to prevent direct panics from crashing the Node.js process.
  doc.rust-lang.org/book/ch09-01-unrecoverable-errors-with-panic.html
- Remove unwrap() at critical points in the resvg-js rendering pipeline.
Err(panic) => Err(Error::RenderPanic(panic_to_string(panic)).into()),
}
}
}
Copy link
Copy Markdown
Member Author

@yisibl yisibl Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If future support for unwind is added to Wasm, it could be implemented this way:

// Share panic-to-error handling across Node.js and wasm targets.
macro_rules! render_inner_catch_unwind_impl {
    ($err_ty:ty) => {
        fn render_inner_catch_unwind(&self) -> Result<RenderedImage, $err_ty> {
            match std::panic::catch_unwind(AssertUnwindSafe(|| self.render_inner())) {
                Ok(result) => result.map_err(Into::into),
                Err(panic) => Err(Error::RenderPanic(panic_to_string(panic)).into()),
            }
        }
    };
}

#[cfg(not(target_arch = "wasm32"))]
impl Resvg {
    render_inner_catch_unwind_impl!(NapiError);
}

#[cfg(target_arch = "wasm32")]
impl Resvg {
    render_inner_catch_unwind_impl!(js_sys::Error);
}

For .cargo/config.toml

[target.wasm32-unknown-unknown]
rustflags = ["-Cpanic=unwind", "-Cllvm-args=-wasm-use-legacy-eh=false"]

[unstable]
build-std = ["std", "panic_unwind"]
@yisibl yisibl merged commit 4b2adf3 into main Jan 27, 2026
37 checks passed
@yisibl yisibl deleted the catch_unwind branch January 27, 2026 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants