throwing object from streaming loader does not work on client navigation

[cloudkite]

I'm using React Router as a...

framework

Reproduction

behaviour observed with both <Await> and use()

using use hook:
https://stackblitz.com/edit/stackblitz-webcontainer-api-starter-gdwvvw2y?file=app%2Froutes%2Fitem-stream.tsx

using Await component:
https://stackblitz.com/edit/stackblitz-webcontainer-api-starter-sn1xvkxt?file=app%2Froutes%2Fitem-stream.tsx

Expected Behavior

When defining a loader that returns a promise that can reject with a custom object:

export function loader({ params }: Route.LoaderArgs) {
  let getItem = async () => {
    await new Promise((resolve) => setTimeout(resolve, 400));
    if (params.itemId !== '1') {
      throw { code: 'NOT_FOUND' };
    }
    return { name: 'hello' };
  };
  return { item: getItem() };
}

I would expect the ErrorBoundary to receive the custom object { code: 'NOT_FOUND' };

Actual Behavior

in the stackblitz example click item stream invalid link

opening a page using client navigation (via a link) that throws from a streamed promise the ErrorBoundary does not receive the custom object but instead receives Error: An unknown error occurred

however opening the same page with full refresh/ server render, produces the expected result:

[electrolyte-orb]

I think that's the problem. Loader haven't thrown yet. It (React comp.) will throw once the promise is resolved or rejected. So it's not a 4xx or 5xx error, it's a React error. Maybe??? idk

[electrolyte-orb]

https://react.dev/reference/react/use

the promise's catch block will be caught by error boundary, i think... or might as well be a bug

[electrolyte-orb]

if we try to error out like this:

the error is actually happening on the client (and that too because of the error in the rendered component):

BUT, on a full page reload, for some reason, the server log says:

so guess it's a bug, BUT NOT THE BUG YOU DESCRIBED IN "EXPECTED BEHAVIOUR" SECTION: i dont think there is a way to send a 404 AFTER the initial HTML (the critical data stream) has been already sent. think about it.... the scaffolding had already been sent, now in the middle of a streaming response, you say: no no, i want a 404. YOU HAVE TO HANDLE IT ON THE CLIENT SIDE

[cloudkite]

Thanks to be clear I wasn’t trying to get the http status code to be 404 as I am aware it’s too late to change the response headers at that point. I want to have a code on the client side so I can show a better error message.

My example would have been better/clearer if it was just

export function loader({ params }: Route.LoaderArgs) {
  let getItem = async () => {
    await new Promise((resolve) => setTimeout(resolve, 2000));
    if (params.itemId !== '1') {
      throw { code: 'NOT_FOUND' };
    }
    return { name: 'hello' };
  };
  return { item: getItem() };
}

{ code: 'NOT_FOUND' } Gets lost/swallowed and turns into Error: An unknown error occurred

[cloudkite]

have updated the example and description to be clearer

[electrolyte-orb]

Have you tried throwing an object of type Error?
Router's docs are very clear about this, you are NOT supposed to be using throw as a means for error handling:

It's not recommended to intentionally throw errors to force the error boundary to render as a means of control flow. Error Boundaries are primarily for catching unintentional errors in your code.

In my opinion the best thing would be to add an additional err key in loader's response and set it to null when there are successful responses. Like return {name: null, error: "NOT FOUND"} and remove the throw statement altogether.

PS. https://reactrouter.com/how-to/error-boundary#2-write-a-bug

This will render the instanceof Error branch of the UI from step 1.

This page makes me think if this is bug??

[cloudkite]

the docs also specifically say throwing for not found is recommended

https://reactrouter.com/how-to/error-boundary#3-throw-data-in-loadersactions

[GabenGar]

Aren't you basically returning a deferred response? In that case you'll have to handle the errors from it the within the errorElement prop of <Await> component with the help of useAsyncError.

[cloudkite]

Aren't you basically returning a deferred response? In that case you'll have to handle the errors from it the within the errorElement prop of <Await> component with the help of useAsyncError.

I tried that doesn't make a difference. As per docs errorElement is optional, if you don't supply an errorElement prop the error will bubble up to the nearest Errorboundary

[brophdawg11]

turbo-stream expects promises to be rejected with Error instances currently: https://github.com/jacob-ebey/turbo-stream/blob/main/src/turbo-stream.ts#L234

the docs also specifically say throwing for not found is recommended

This is more intended for critical data where you want to return a 404 status code. For an item page like /item/1 where the item is the critical piece of UI we would usually recommend not-deferring that data so you can use appropriate status codes. Deferred data works better for non-critical (lazy) data - such as custom reviews of the item for example.

I don't know if there' appetite to open up rejections with custom objects, but for now I would update to throw an Error instance.