feat: add a server function error handler - #2262
Conversation
✅ Deploy Preview for solid-start-landing-page ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 38078e8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
f25d012 to
acccfe0
Compare
| // Nothing else observes a server function failure: it is caught here and | ||
| // written straight to the response, so monitoring never sees the crash and | ||
| // the thrown value reaches the client as-is. | ||
| x = globalThis.__transformServerFnError?.(x) ?? x; |
There was a problem hiding this comment.
I don't agree with a globalThis implementation, that can be tampered with by another code (hoping its not malicious actors)
There was a problem hiding this comment.
Agreed, dropped it. The handler is now registered through setServerFunctionErrorHandler, exported from @solidjs/start/fns/server, so nothing reads or writes globalThis.
4f304fb to
2d14f17
Compare
2d14f17 to
ce68392
Compare
| export function setServerFunctionErrorHandler( | ||
| handler: ServerFunctionErrorHandler | undefined, | ||
| ): void { | ||
| errorHandler = handler; |
There was a problem hiding this comment.
Functionally, I think it's still the same as the last one since another package can just import this function and call it through peer dependency.
I think the go-to solution here is somewhere similar to #1474 , I think we can change that where instead of just exporting the plugins, we can also export something like an onServerFunctionError that the runtime can call.
Example:
// serialization.ts
export function onServerFunctionError(error: unknown) {
return doStuff(error);
}import { onServerFunctionError } from 'solid-start:serialization';
// ...
x = onServerFunctionError(err);
PR Checklist
What is the current behavior?
handleServerFunctioncatches everything a server function throws and writes it straight into the response. Nothing else observes it, so the crash is invisible to monitoring, and whatever was thrown reaches the client as-is.What is the new behavior?
A new
serverFunctions.onErroroption names a module whose default export is called with the thrown value before it is serialized. Returning a value replaces what is sent, returningundefinedchanges nothing.Naming the module in the config rather than registering a handler at runtime keeps the app in sole control of it, so no dependency can reach the running server and take over reporting. It follows
serialization.pluginsfrom #1474, with its own virtual module because this one is bundled into the server only and may import a monitoring SDK.How other frameworks do this:
handleError, which observes and replaces: docs (src). Samehandler(...) ?? fallbackshape as this PR.onRequestError, observe only: docs (src)handleError, observe only: docs (src)errorhook, observe only: docs (src)functionMiddlewarewraps every server function, so an app catches aroundnext()itself: docs (src)Replacing matters for adapters whose runtime wraps failures, such as Effect's
FiberFailure. Unwrapping here keeps aResponsethrown through the wrapper recognised as control flow by thex instanceof Responsebranch.