RFC: Standardise ServerErrors

[shairez]

Discussed in #200

^{Originally posted by DustinJSilk December 15, 2024}

What is it about?

ServerErrors should not return a value on the client, they should be thrown, and the error types should be the same for routeLoaders

What's the motivation for this proposal?

Problems you are trying to solve:

• Throwing a ServerError in a server$ function allows us to change the status code and payload of the repsonse. On the client, the value of the error is returned, rather than having the client throw the error so that it can be caught. This brings about a few challenges:
  • This breaks JS norms. It is expected that throwing an error means the error should be caught, instead, it is magically caught and the value is returned by the server$ functions.
  • This means awkward types must be used as the server$ function has to return a tuple to handle the error path. See the original PR for an example: feat(server$): config argument, optional GET, ServerError qwik#6290
  • The user has to handle 2 types of errors now: error values and thrown errors such as transient errors. Thus a try/catch is still needed along with the error value response.
  • Updating a server$ function which previously might throw a standard error now also requires changes to how the function is called due to the change in it's signature with a tuple response
• A routeLoader$ throws an error using the syntax throw event.error(500, 'some data'). This has 2 significant issues:
  • If we are running middleware to correctly set http status codes, we cannot simply throw a ServerError as the routeLoader$ expects a different error instance to the server$.
  • Throwing an error is only possible when you have access to the event object, which makes implementing middleware more difficult.

Goals you are trying to achieve:

• Standardise the error classes across server functions.
• Handle server$ errors as if they are standard errors on the client.

Any other context or information you want to share:

This would be a breaking change, but I dont think this API is widely used currently considering it doesn't work in a dev build due to this error: QwikDev/qwik#7165
This means we might be able to implement this change as a minor version bump, although it would technically be a breaking change.

---

Proposed Solution / Feature

What do you propose?

• Standardise the server$ and routeLoader$ error types to both use a ServerError class which isn't part of the event object.
• Have the client throw ServerErrors rather than returning the error payload as if its a successful response

Code examples

const useData = routeLoader$(() => {
  throw ServerError(500, 'some data')
})

const getData = server$(() => {
  throw ServerError(500, 'some data')
})

export default component$(() => {
  useVisibleTask(() => {
    getData()
      .then()
      .catch(err => {
        // handle error
      }) 
  })
}) 

---

Links / References

Original ServerError PR: QwikDev/qwik#6290

[DustinJSilk]

Here's a PR tracking this work: QwikDev/qwik#7185

Some remaining tasks:

• Add tests to ensure @plugin middleware can intercept errors thrown in server$ and routeLoader$
• Remove ErrorResponse handling
• Replace the error() function on the event object to return a ServerError to maintain backwards compatibility
• Add a documentation page on error handling
  • Overview on the ServerError (generic type for serialised response data + custom status codes + how to throw and catch)
  • Adding structured error logs with a plugin
  • Hiding errors on production (intercept and return a generic error response)
  • Using the event.error() helper functions simply create a ServerError instance

This change would also remove the error log here. Since errors can be intercepted by middleware this should be left to the user. When errors arent intercepted, they're returned to the frontend by default. If this is difficult for new comers facing their first errors, we can add a logging plugin to the starter templates.

@shairez please let me know if Ive missed anything.