feat(server$): config argument, optional GET, ServerError

[PatrickJS]

Overview

fixes #5656
fixes #6288

[cloudflare-workers-and-pages]

Deploying qwik-docs with    Cloudflare Pages

Latest commit:	fac3aaa
Status:	✅  Deploy successful!
Preview URL:	https://4077d306.qwik-8nx.pages.dev
Branch Preview URL:	https://feat-server--options.qwik-8nx.pages.dev

View logs

[PatrickJS]

in another pr I can add 3 levels of config. I also want to do this for better AbortSignal

[DustinJSilk]

This is awesome! 🤩

Would it make sense to have ServerError take a generic argument so any serializable data can be returned?

export class ServerError<T = Record<any, any>> extends Error {
  constructor(
    public status: number,
    public data: T
  ) {
    super();
  }
}

[wmertens]

Looking forward to this!

[wmertens]

Misko is ok with this API extension

done

[PatrickJS]

import { server$ } from "@builder.io/qwik-city";
import { ServerError } from "@builder.io/qwik-city/middleware/request-handler";

type ResponseTuple = [null | string, string];

const serverFunctionA = server$(async function a(): Promise<ResponseTuple> {
  throw new ServerError<[string]>(401, ["my error"]);
});

const serverFunctionB = server$(async function b(): Promise<ResponseTuple> {
  return [null, this.method || ""];
});

export const MultipleServerFunctionsInvokedInTask = component$(() => {
  const methodA = useSignal("");
  const methodB = useSignal("");

  useVisibleTask$(async () => {
    const [error /*, data */] = await serverFunctionA();
    if (error) {
      methodA.value = error;
    }
    await delay(1);
    //     err, method
    const [, method] = await serverFunctionB();
    methodB.value = method;
  });

  return (
    <div id="server-error">
      {methodA.value}
      {methodB.value}
    </div>
  );
});

[DustinJSilk]

I'm curious why the error isn't thrown on the client so that it can be caught, and instead we return the error like the method was successful?

I'm wondering if this was the right decision for the given reasons:

• Does this go against usual JS conventions of catching errors rather than returning errors?
• Throwing an error and then not catching it later feels wrong - throwing should mean we catch
• We now have to handle both transient errors and ServerErrors - if the request fails we need to catch it, and if a ServerError is thrown we need to handle the value type.
• Changing code to throw ServerError instead of a native error means we have to update our code elsewhere to handle the new error response type, making it more of a breaking change

There is also actually a ErrorResponse class from a routeLoader$, which it might have made sense to reuse so that any API middleware, whether used in a server$ or routeLoader$, can throw the same class and result in the same status code/response handling.

I realise this might be late, but with the upcoming v2 changes, would it make sense to revisit this, or is there merit in the way it is currently handled?

EDIT
I've created an RFC to have this changed: QwikDev/qwik-evolution#200