plugins: isolate an activate() failure instead of crash-looping the server (#645) - #646
Open
melvincarvalho wants to merge 1 commit into
Open
melvincarvalho wants to merge 1 commit into
melvincarvalho wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
…erver A host reboot brought jspod back under pm2 before the Bitcoin node one of its plugins talks to. That plugin's activate() fetches the node's RPC, got "fetch failed", the loader rethrew, the server exited 1, pm2 restarted it, and the cycle ran ~290 times. For those ten minutes nothing on the pod answered: not the other three plugins, not plain LDP serving. A client that normally writes through the pod fell back to direct disk writes and corrupted a file in a concurrent-write race. None of that had anything to do with the plugin that failed. The fail-loud rule exists so a server never silently boots without an app the operator configured. That is the right call for a plugin that cannot be LOADED (bad entry, unresolvable module, no activate export, invalid prefix, duplicate id) and for the api's own validation errors (malformed reservePath, bad mountApp prefix, a path two plugins claim): those are mistakes in the config or the plugin, and they stay fatal. An activate() that throws for any other reason is a different animal: the usual cause is a dependency that is not up yet, which is exactly the state after a reboot. So by default an activate() failure is now isolated. The error is logged with the plugin id, the plugin is skipped, every method under its prefix answers 503 with Retry-After: 30 and a JSON body naming the plugin and the reason, the boot continues, and the load ends with a "N active, M skipped" warning. The outcome is also readable off the server as fastify.pluginFailures. Routes the plugin registered before throwing are left in place (Fastify has no unregister); the placeholder claims only what is still free. Re-activation needs a restart, because the route table is sealed once listening; the 503 makes that a visible, retryable state rather than a dead server. Operators who would rather not start at all than start without an app opt back into the old behaviour with pluginStrict (--plugin-strict, JSS_PLUGIN_STRICT=true). Closes #645
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #645.
Problem
One plugin's activate() threw (fetch failed: its Bitcoin node wasn't up yet after a host reboot). The loader rethrows, the server exits 1, pm2 restarts it, repeat: ~290 restarts in ten minutes, during which the other plugins and plain pod serving were all down too. A client that writes through the pod fell back to direct disk writes and corrupted a file in a race.
Change
Default: isolate. When activate() throws, the loader now logs the error with the plugin id, skips the plugin, mounts a 503 placeholder (Retry-After: 30, JSON body { error, plugin, reason }) at the prefix and prefix/* for every method, and continues. The load ends with a plugins: N active, M skipped after activate() failed: … warning, and the list is exposed as fastify.pluginFailures ([{ id, prefix, module, error }]).
Still fatal, in every mode: anything that is a config or plugin-author mistake rather than an unavailable dependency. Load errors (bad entry, unresolvable module, no activate, invalid prefix, duplicate id) are unchanged. The api's own validation errors (reservePath shape/collision, mountApp handler/prefix) are now tagged with a private symbol and rethrown by the activate() catch, so the existing reservePath/mountApp "fails the boot" tests keep passing without pluginStrict.
Opt-out: pluginStrict: true / --plugin-strict / JSS_PLUGIN_STRICT=true restores today's fail-fast boot.
Limits
Tests
Two new cases in test/plugins.test.js: a throwing fixture alongside a healthy one (sibling up, pod up, 503 shape at root/subtree/other method, pre-throw route kept, pluginFailures contents), and pluginStrict rejecting listen() with the old message. Full suite: 1154 pass.
Docs: new "When a plugin fails" paragraph in docs/configuration.md; loader header comment updated.