fix: server$ error handling

.changeset/lucky-lies-exist.md

@@ -0,0 +1,5 @@
+---
+'@builder.io/qwik-city': patch
+---
+
+server$ functions now correctly throw 4xx errors on the client and server$ errors can be caught by @plugin middleware

packages/qwik-city/src/middleware/request-handler/resolve-request-handlers.ts

@@ -12,7 +12,6 @@ import type {
   RouteModule,
   ValidatorReturn,
 } from '../../runtime/src/types';
-import { ServerError } from './error-handler';
 import { HttpStatus } from './http-status-codes';
 import { RedirectMessage } from './redirect-handler';
 import {
@@ -305,23 +304,12 @@ async function pureServerFunction(ev: RequestEvent) {
       const [qrl, ...args] = data;
       if (isQrl(qrl) && qrl.getHash() === fn) {
         let result: unknown;
-        try {
-          if (isDev) {
-            result = await measure(ev, `server_${qrl.getSymbol()}`, () =>
-              (qrl as Function).apply(ev, args)
-            );
-          } else {
-            result = await (qrl as Function).apply(ev, args);
-          }
-        } catch (err) {
-          if (err instanceof ServerError) {
-            ev.headers.set('Content-Type', 'application/qwik-json');
-            ev.send(err.status, await qwikSerializer._serializeData(err.data, true));
-            return;
-          }
-          ev.headers.set('Content-Type', 'application/qwik-json');
-          ev.send(500, await qwikSerializer._serializeData(err, true));
-          return;
+        if (isDev) {
+          result = await measure(ev, `server_${qrl.getSymbol()}`, () =>
+            (qrl as Function).apply(ev, args)
+          );
+        } else {
+          result = await (qrl as Function).apply(ev, args);
         }
         if (isAsyncIterator(result)) {
           ev.headers.set('Content-Type', 'text/qwik-json-stream');

packages/qwik-city/src/middleware/request-handler/user-response.ts

@@ -1,7 +1,12 @@
 import type { QwikSerializer, ServerRequestEvent, StatusCodes } from './types';
 import type { RequestEvent, RequestHandler } from '@builder.io/qwik-city';
-import { createRequestEvent, getRequestMode, type RequestEventInternal } from './request-event';
-import { ErrorResponse, getErrorHtml, minimalHtmlResponse } from './error-handler';
+import {
+  RequestEvQwikSerializer,
+  createRequestEvent,
+  getRequestMode,
+  type RequestEventInternal,
+} from './request-event';
+import { ErrorResponse, ServerError, getErrorHtml, minimalHtmlResponse } from './error-handler';
 import { AbortMessage, RedirectMessage } from './redirect-handler';
 import type { LoadedRoute } from '../../runtime/src/types';
 import { encoder } from './resolve-request-handlers';
@@ -72,6 +77,13 @@ async function runNext(requestEv: RequestEventInternal, resolve: (value: any) =>
         const status = e.status as StatusCodes;
         requestEv.html(status, html);
       }
+    } else if (e instanceof ServerError) {
+      if (!requestEv.headersSent) {
+        const status = e.status as StatusCodes;
+        const qwikSerializer = requestEv[RequestEvQwikSerializer];
+        requestEv.headers.set('Content-Type', 'application/qwik-json');
+        requestEv.send(status, await qwikSerializer._serializeData(e.data, true));
+      }
     } else if (!(e instanceof AbortMessage)) {
       if (getRequestMode(requestEv) !== 'dev') {
         try {

packages/qwik-city/src/runtime/src/server-functions.ts

@@ -479,19 +479,19 @@ export const serverQrl = <T extends ServerFunction>(
         } else if (contentType === 'application/qwik-json') {
           const str = await res.text();
           const obj = await _deserializeData(str, ctxElm ?? document.documentElement);
-          if (res.status >= 500) {
+          if (res.status >= 400) {
             throw obj;
           }
           return obj;
         } else if (contentType === 'application/json') {
           const obj = await res.json();
-          if (res.status >= 500) {
+          if (res.status >= 400) {
             throw obj;
           }
           return obj;
         } else if (contentType === 'text/plain' || contentType === 'text/html') {
           const str = await res.text();
-          if (res.status >= 500) {
+          if (res.status >= 400) {
             throw str;
           }
           return str;

starters/apps/qwikcity-test/src/routes/(common)/server-func/server-error/index.tsx

@@ -3,28 +3,32 @@ import { server$ } from "@builder.io/qwik-city";
 import { ServerError } from "@builder.io/qwik-city/middleware/request-handler";
 import { delay } from "../../actions/login";
 
-type ResponseTuple = [null | string, string];
+type ErrorReason = { reason: string };
 
-const serverFunctionA = server$(async function a(): Promise<ResponseTuple> {
-  throw new ServerError<[string]>(401, ["my error"]);
+const serverFunctionA = server$(async function a(): Promise<string> {
+  throw new ServerError<ErrorReason>(401, { reason: "my error" });
 });
 
-const serverFunctionB = server$(async function b(): Promise<ResponseTuple> {
-  return [null, this.method || ""];
+const serverFunctionB = server$(async function b(): Promise<string> {
+  return this.method;
 });
 
 export const MultipleServerFunctionsInvokedInTask = component$(() => {
   const methodA = useSignal("");
   const methodB = useSignal("");
 
   useVisibleTask$(async () => {
-    const [error /*, data */] = await serverFunctionA();
-    if (error) {
-      methodA.value = error;
+    try {
+      await serverFunctionA();
+    } catch (err: any) {
+      if (isErrorReason(err)) {
+        methodA.value = err.reason;
+      }
     }
+
     await delay(1);
-    //     err, method
-    const [, method] = await serverFunctionB();
+
+    const method = await serverFunctionB();
     methodB.value = method;
   });
 
@@ -43,3 +47,11 @@ export default component$(() => {
     </>
   );
 });
+
+function isErrorReason(err: any): err is ErrorReason {
+  if (typeof err.reason === "string") {
+    return true;
+  }
+
+  return false;
+}