Standardise ServerErrors

[DustinJSilk]

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

[wmertens]

Right, that does seem more intuitive. v2 is a good opportunity to make this breaking change @mhevery

[mhevery]

I agree

[shairez]

@DustinJSilk thanks for the proposal!
We discussed this in the latest RFC meeting and decided it's a good idea

Want to help us and champion it?

[DustinJSilk]

You’re welcome!

Sure, I’m happy to champion it. I’ll start the RFC soon

[shairez]

Thanks!
I've promoted this into an RFC issue.
sent you an invite to the code contributors, once you accept it we could assign it to you