[Feature?]: Error Handling with solid-router

[LexSwed]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Summary 💡

Allow for semantic error handling with server actions.
This is a continuation of solidjs/solid-router#365 discussion, but focused on solid-start server actions.

The goal is to allow:

• return non-200 (401, 500, etc) responses from server actions for observability platforms to monitor how many responses are not 200.
• handle such responses in code with reactive useSubmission().error when using without the form.

Proposal: use Response.ok to populate useSubmission().error.

const serverAction = action(() => {
  'use server';
  try {
    return perform();
  } catch(error) {
    // Error is serialised, no queries are revalidated (unless specified otherwise)
    // Response.ok is `false`, so `submission.error` is serialised Error.
    // This will make the action use to fail (as the error is rethrown), so needs try/catch
    return json(new Error('Something went wrong'), { code: 500 });
  }
});

const Component = () => {
  const performAction = useAction(serverAction);
  const submission = useSubmission(serverAction);

  // See example, QR Code scanning
  const userActionWithoutForm = () => {
    try {
      await performAction(); // same as fetch().catch() when the request has failed
    } catch {}
  }

  return (
    <Switch>
      <Match when={submission.pending}>Checking</Match>
      <Match when={submission.error}>Retry with another input</Match>
      <Match when={true}>Perform action</Match>
    </Switch>
  )
}

If the action returns non-Error data, the action does not fail:

// action, does not throw an Error
  return json('Something went wrong', { code: 500 });
// Component
  // similar to fetch(response => response.json()) without checking for response.ok
  await performAction();
  // rely on submission.error being populated with 'Something went wrong' string

Examples 🌈

In the app, users can scan a QR code, when valid code is scanned – action is executed. If scanned code is invalid – error message is shown with Retry logic:

const checkCode = action(async code => {
  "use server";
  try {
    return validity(code);
  } catch(error) {
    // ===Unclear===
  }
});
// component:
const submission = useSubmission(checkCode);
const checkCode = useAction(checkCode);

const onScanned = code => {
  try {
    await checkCode(code);
  } catch(error) {
    // do something, or just ignore it, same way we would do with using fetch directly.
  }
}

return (
  <Switch>
    <Match when={submission.pending}>Checking</Match>
    <Match when={submission.error}>Wrong code</Match>
    <Match when={true}><QRScanner onSuccess={onScanned} /></Match>
  </Switch>
);

Motivation 🔦

Right now, to return non-200 response from a server action, one needs to use custom Response:

'use server';
return Response({ error: 'Something went wrong' }, { code: 500 });

This, however, makes useSubmission(action).error to be empty, because the response is a valid Response:

https://github.com/solidjs/solid-router/blob/e773947b85ac78281816e86621a2cdb6735ae95a/src/data/action.ts#L149

Making the data type from the action ambiguous: { correctResponse } | { error }, which makes handling of the response complicated:

const performAction = useAction(serverAction);
const submission = useSubmission(serverAction);
const [error, setError] = createSignal(null);
const someUserAction = () => {
  try {
    const data = performAction();
    if (data.error) {
    // do nothing, rely on submission.result.error
    // but submission.error !== submission.result.error
    // by design to distinguish failures?
    } else {} // success logic
  } catch(error) {
    // will never get here
  }
}

So, another option is to make sure submission.error is populated, throw an error from the action.

const serverAction = action(() => {
  'use server';
  throw new Error('Something went wrong');
});

This works for useSubmission().error, but the HTTP Response status is now 200, so one needs custom metrics to monitor how the API is performing.

Doing return json(new Error('error'), { status: 500, revalidate: [] }) is making action to fail itself (?). So 500, but all queries are revalidated (?) and submission.error empty.

[ryansolid]

Relevant discussion: solidjs/solid-router#387

[frenzzy]

Currently, I have to wrap up all my server actions to catch unexpected errors for the same reason, in the following manner:

// createAction.ts
export function createAction<T extends any[], U = void>(
  fn: (...args: T) => Promise<U>,
) {
  return async (...args: T): Promise<U> => {
    try {
      return await fn(...args)
    } catch (err) {
      // Thrown redirect
      if (err instanceof Response) return err as never

      // Safe known error (from valibot)
      if (err instanceof ValiError) return err as never // TODO: set 400 http status code

      // Send runtime error to monitoring system
      console.error(err)

      // Send a sanitized error message to client-side
      return new Error('Internal Server Error') as never // TODO: set 500 http status code
    }
  }
}

// doSomething.ts
"use server"
import { createAction } from './createAction'
export const doSomething = createAction(async (formData: FormData) => {
  // action logic
})

// component.ts
import { doSomething as _doSomething } from './doSomethig'
const doSomething = action(_doSomething)

With this approach, I still do not have the ability to set custom HTTP status codes, but at least I can monitor unhandled runtime server-action errors. I am also forced to always extract server action code into a separate file, which is not necessarily bad, just a conclusion.

I still believe that SolidStart should provide special middleware support for server actions to avoid wrapping all actions manually in userland and to extend functionality—for example, to return custom status codes as requested by this issue.

---

Also related to actions: there may be a mistake in this code: https://github.com/solidjs/solid-router/blob/e773947b85ac78281816e86621a2cdb6735ae95a/src/data/action.ts#L179

The client-side should actually receive null if an action explicitly returns it. Currently, the server action response appears empty in such cases, which looks weird (not obvious behavior).

[ryansolid]

The reason I bring up the other discussion is this was looked at at length. And I don't want to restart the discussion from blank slate here. There are a lot of details. Where it landed was we would propagate the error field on specifically thrown things and that we would rethrow when not used in a form.

My understanding of the gap right now is that there is no way to both throw and set the HTML status code. I was tempted to make all thrown errors 500s but I think that assumes too much? In general throwing doesn't mean an error in Server functions, and serializing an error doesn't mean they failed.

But generally speaking using .error should be avoided. It can't be typed (because it is unexpected) and it is designed to handle things we aren't aware of. I guess the difference between our solutions and others (Remix/Next) is they would throw in these cases to an error boundary and that is difficult the way things are setup, because there is no connection to the render tree for forms.

The complaint I'm gathering is that having to account for .error is extra work. Like the idea here is:

function SomeComponent() {
  const submission = useSubmission(myAction);
  
  return <Show when={!submission?.error} fallback={<UnknownError />}>
    {/* Content */}
    <Show when={submission.result}>
      {/* Other Error Stuf */}
    </Show>
  </Show>
}

Like treat .error only as almost an ErrorBoundary, like when you have no clue what happened. Largely this is why I don't want to rely on res.ok as server functions outside of actions are perfectly fine throwing things. It is just a transparent communication.

I'm having a hard time seeing how to handle all the cases outside of the current solution.

[LexSwed]

I was tempted to make all thrown errors 500s but I think that assumes too much?

Worth checking with other devs and frameworks (maybe even including outside of JS ecosystem). If code throws and not handles the error – is it not a server error? IMO every error should be handled on some level, and generic error re-thrown to not expose error message to the client. Then, assuming that an unexpected error is 500 seems correct.

In general throwing doesn't mean an error in Server functions

Unless it's an instanceof Error (or custom error).

using .error should be avoided. It can't be typed

It's same as try/catch, one has to check the type of the thrown value, it's expected from TS devs.

treat .error only as almost an ErrorBoundary, like when you have no clue what happened.

I thought of .error as "request didn't succeed, handle possible reasons in ways that make sense", so handling specific cases (known error) and generic cases (unknown error).
And <ErrorBoundary> as "error was not handled in the code itself, something completely unexpected happened that makes user flow impossible to recover, show fallback UI to restart the flow".

[LexSwed]

Trying to take a step back.

const myAction = action(code => {
  try {
    const result = await logic(code);
    return result; // or json(result);
  } catch(error) {
    return json({ error: 'code-5' } as const, { status: 503 });
  }
}, 'my-action');

The return type is typeof result or { error: 'code-5' }.

action can be used in 2 ways, as useAction(myAction) as <form action={myAction}>.

In case of <form action={myAction}>, if the action has failed (error was thrown OR non-200 response), the developer is expected to handle the error via submission.error (Note: can they use ErrorBoundary in this case?)

When the action is executed on demand (f.e. 3rd party calls) with const execute = useAction(myAction), like:

<QRScanner onSuccess={code => {
  const result = execute(code);
  // next steps
}} />

should they wrap execute(code) into try/catch? If yes, what's the purpose of catch?

<QRScanner
  onSuccess={(code) => {
    try {
      const result = execute(code);
      // next steps
    } catch (error) {
      // what is error here? should it be deserialized body?
      // or fetch() Response? Then, people can access error.ok or error.json() or whatnot
      // but fetch() doesn't `catch` when request didn't fail, it just has response.ok === false
    }
  }}
/>

Then, basically, all execute() calls are successful unless request itself has failed, mimicking native fetch:

<QRScanner
  error={submission.error} // { error: 'code-5' } on failure
  onSuccess={(code) => {
      const result = execute(code);
      if ('error' in result) {  // { error: 'code-5' }
        // request failed - decide yourself if this branch needs any logic
        // maybe sometimes this needs additional handling, like opening a popup or showing a notification
      }
      // next steps
  }}
/>

This looks complete to me:

• UI can handle errors returned by the actions via submission.error, narrowing the types manually if necessary
• code execution can handle errored code as well

[ryansolid]

@LexSwed I agree that the last post sounds complete to me. Are we missing anything for this? I think you are describing how it works right now.

<QRScanner
  onSuccess={(code) => {
    try {
      const result = execute(code);
      // next steps
    } catch (error) {
      // what is error here? should it be deserialized body?
      // or fetch() Response? Then, people can access error.ok or error.json() or whatnot
      // but fetch() doesn't `catch` when request didn't fail, it just has response.ok === false
    }
  }}
/>

To answer your question. We generally don't error here unless the function throws on the server or the fetch request fails. So I'm gathering it is the error object. (ie the deserialized body). Although in your example above where you catch inside the server action this catch would never be hit unless fetch failed. Which yes aligns with your last code snippet.

[LexSwed]

The difference is in submission. Right now error is only set when

the function throws on the server or the fetch request fails

which means that developer has to take care of different errors differently:

<Switch>
  <Match when={submission.error}>Fetch or function throws</Match>
  <Match when={'error' in submission.result && submission.result.error}>Own error, could be multiple different ones</Match>
</Switch>

While if solid was setting useSubmission().error to any response body that isn't 200 (response.ok):

<Switch>
  // error is the body from json({ errorCode: 5 | 10 }, { status: 422 })
  <Match when={submission.error?.errorCode === 5}>Fetch or function throws</Match>
  <Match when={submission.error?.errorCode === 10}>Own error, could be multiple different ones</Match>
</Switch>

Which is probably pain to type from solid-router perspective and does not give much type safety (submission.error is still any).

There might be a need for the error handling best practice in solid-start, to draw the line what submission.error means. Because if it's scoped to just fetch request fails or unexpected error, then it's understandable, it's like fetch().catch(). But if the user is allowed to throw Error from the action and handle it in templating, then error handling becomes ambiguous:

• throw from actions and handle errors via submission.error? Sure, but you can't specify the response status code (internal error vs validation error).
• return json({ error: 'something' }, { status: 422 })? yes, but now handle both, submission.error and 'error' in submission.result && submission.result.error (TypeScript). Which is probably fine, as it makes it easier to understand what action can "throw" as it forces to type narrow the result type.

Not sure anymore tbh. Maybe the solution is just discouraging throwing from the actions, and treat submission.error only as unhandled error that always means 500 and that's it.

[frenzzy]

I've addressed the concern regarding the inability to set custom status codes by implementing the following solution:

// utils.ts
import { json } from '@solidjs/router'
import { isDev } from 'solid-js/web'

export class ValidationError extends Error {
  constructor(message: string) {
    super(message)
    this.name = 'ValidationError'
  }
}

export const actionMiddleware = <T extends any[], U = void>(
  fn: (...args: T) => Promise<U>,
) => {
  return async (...args: T): Promise<U> => {
    try {
      return await fn(...args)
    } catch (err) {
      // Thrown redirect
      if (err instanceof Response) return err as never

      // https://github.com/solidjs/solid-start/blob/v1.0.0-rc.1/packages/start/src/runtime/server-handler.ts#L223
      const xError =
        err instanceof Error
          ? err.message
          : typeof err === 'string'
            ? err
            : 'true'

      // Validation error
      if (err instanceof ValidationError) {
        return json(err, {
          status: 400,
          headers: { 'x-error': xError },
        }) as never
      }

      // TODO: Use logger to send error to monitoring service
      console.error(err)

      // Critical runtime error with sanitized message for production
      return json(isDev ? err : new Error('Internal Server Error'), {
        status: 500,
        headers: { 'x-error': xError },
      }) as never
    }
  }
}

This allowed me to keep only business logic with validation inside server functions without the need to repetitively use try catch:

// server.ts
'use server'

import { redirect } from '@solidjs/router'
import { actionMiddleware, ValidationError } from './utils'
import { db, users } from './db'

export const register = actionMiddleware(async (formData: FormData) => {
  const username = String(formData.get('username'))
  const password = String(formData.get('password'))
  if (!username) throw new ValidationError('Username is required')
  if (!password) throw new ValidationError('Password is required')
  await db.transaction(async (trx) => {
    const [existentUser] = await trx
      .select({ id: users.id })
      .from(users)
      .where(eq(users.username, username))
    if (existentUser) throw new ValidationError('Username is already taken')
    const [user] = await trx
      .insert(users)
      .values({ username, password })
      .returning({ id: users.id })
    if (!user) throw new Error('Failed to create a new user') // internal error
    const session = await getSession()
    await session.update((data) => {
      data.userId = user.id
    })
    return redirect('/')
  })
})

And it seems to work fine with both submission.error and action().catch:

import { action, useAction, useSubmission } from '@solidjs/router'
import { register } from './server'

const registerAction = action(register);

const RegistrationForm = () => {
  const action = useAction(registerAction);
  const submission = useSubmission(registerAction);

  const perform = async (event) => {
    try {
      await action(new FormData())
    } catch (err) {
      console.error(err)
    }
  }

  return (
    <form action={register} method="post">
      <input type="text" name="username" />
      <input type="password" name="password" />
      <button type="submit">Send Form</button>
      <button type="button" onClick={perform}>Perform Action</button>
      <p>submission.error: {JSON.stringify(submission.error)}</p>
    </form>
  )
}

It would be great if SolidStart handled HTTP status codes in a similar manner under the hood. In that case, we would need to reintroduce a special error type similar to the one exported before v0.4: FormError.tsx or implement a mechanism for injecting middleware into server functions, which, in my opinion, is preferable as it allows developers to decide how to handle different types of errors and which ones to log.