fix: catch syntax errors in loaders to prevent blocking app startup - #22
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
- Switch generated barrel and plugins-root modules from static imports to dynamic import() with .catch() so a syntax error in one service file doesn't cascade and block the entire app from loading - Wrap loadNativeStrippedTs (TS type stripping) in try/catch, returning an empty module on failure - Wrap nextLoad + appendAutoRegister for service files in try/catch - All caught errors are logged via console.error with [zenbu-loader] prefix Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| function buildSafeSource(imports: string[]): string { | ||
| return imports | ||
| .map( | ||
| (specifier) => | ||
| `await import(${JSON.stringify(specifier)}).catch(e => console.error("[zenbu-loader] failed to load module", ${JSON.stringify(specifier)} + ":", e.message ?? e))\n`, | ||
| ) | ||
| .join(""); | ||
| } |
There was a problem hiding this comment.
📝 Info: Barrel imports now load concurrently instead of sequentially
The switch from static import to Promise.allSettled([import(...), ...]) means service files within a single plugin barrel now load concurrently rather than in the sequential order determined by the glob expansion. Previously, buildSource(imports) generated sequential static imports (import "a"\nimport "b") which are evaluated in dependency order. Now all imports fire simultaneously. This shouldn't cause issues because: (1) the runtime.register() mechanism at packages/core/src/runtime.ts:237-280 handles registrations independently per service key, (2) services declare explicit deps for ordering, and (3) evaluate() is called later during reconciliation, not at import time. However, if any plugin relies on implicit evaluation order of service file side effects beyond runtime.register, that assumption would break.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| return { | ||
| source: `${buildSource(imports)}import.meta.hot?.accept()\n`, | ||
| source: `${buildSafeSource(imports)}import.meta.hot?.accept()\n`, |
There was a problem hiding this comment.
🚩 Dynamic imports may affect dynohot's module graph pruning for removed service files
With static imports, dynohot can see the barrel's dependencies in the AST and track when a dependency is dropped (e.g., a service file deleted). With dynamic import(), the barrel-to-service relationship is invisible to dynohot's static analysis. Service files DO self-manage HMR via runtime.register() calling import.meta.hot.accept() and import.meta.hot.prune() (packages/core/src/runtime.ts:272-277), so individual file edits are handled correctly. However, the pruning path for a removed file depends on dynohot detecting that the module is no longer referenced. If dynohot relies on the static import graph for pruning decisions, a removed service file's prune callback might not fire, leaving a stale registration in the runtime. The existing deleted-file guard at packages/core/src/loaders/zenbu.ts:579-586 mitigates this for the reload case, but the unregister path through prune may be affected. Worth verifying with a manual test: delete a service file and confirm the service is properly unregistered.
Was this helpful? React with 👍 or 👎 to provide feedback.
…ope error swallowing to service files only - Use Promise.allSettled instead of sequential await import() to preserve parallel module loading behavior - Only catch/swallow errors for service files in the native TS loader path; non-service files propagate errors naturally so developers see clear diagnostics Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This branch was successfully deployed
1 participant
Summary
A syntax error in a single service file previously crashed the entire module import chain, preventing the app from loading at all. This PR makes loading resilient so broken modules are skipped with a logged error.
Two layers of protection:
1. Generated barrel/root modules now use dynamic
import()with.catch()Previously
buildPluginBarrelandbuildPluginsRootemitted staticimport "..."statements — if any module in the chain threw, it cascaded up and killed the app. Now service-file and barrel imports use:The registry import stays static since it's generated code that must succeed.
2.
loadImplfile:// branch catches loader-phase errorsloadNativeStrippedTs(stripTypeScriptTypes) → try/catch, returnsexport {}on failurenextLoad+appendAutoRegisterfor service files → try/catch with async.catch()for promise-based loadersBoth paths log
[zenbu-loader] failed to load <path>: <message>and return an empty module so the rest of the app continues.Link to Devin session: https://app.devin.ai/sessions/f973737f254e4f07aea5ee9ac3f80e53
Requested by: @RobPruzan