Throw error when reading json response fails

[8ctavio]

In some functions, errors thrown by the expression response?.clone.json() are supressed and the response object is returned instead. I'm not sure if this is done because some action should always be taken with the response object in case it can't be cloned or its response can't be parsed as json, but I currently handle the response of this functions in the following way:

const result = await nile.fn()
if (result instanceof Response) {
  // Create error object with result and forward to error handler.
}

This could be done automatically if an error was thrown instead. Perhaps it is a better option to throw in case of an error, and users can catch if needed. The response object itself could be thrown, but a custom Error object could also be created.

For reference, this are the places where response objects are returned upon catching an error:

• https://github.com/niledatabase/nile-js/blob/v5.3.1/packages/server/src/tenants/index.ts#L76
• https://github.com/niledatabase/nile-js/blob/v5.3.1/packages/server/src/tenants/index.ts#L203
• https://github.com/niledatabase/nile-js/blob/v5.3.1/packages/server/src/tenants/index.ts#L468
• https://github.com/niledatabase/nile-js/blob/v5.3.1/packages/server/src/auth/index.ts#L83
• https://github.com/niledatabase/nile-js/blob/v5.3.1/packages/server/src/auth/index.ts#L129

[jrea]

My original intent here was to not have an opinion on dealing with errors, are sometimes there's useful stuff from result.text() that's surfaced out of the API.

Does it make sense to just throw Error with that text? Just have to go though the API and be sure we're doing that. I think we are (famous last words).

edit: Oh, I also realize that it could be a 401 (no text) and there's no mechanism to surface that. We could do all custom errors for that kind of stuff, but right now the onus is on developers to handle the myriad of failures that could be happening (even fetch throwing). At least a Response object is a standard thing most people are going to know about

[8ctavio]

If the objective is to surface the response object upon encountering error, perhaps the following is a good alternative?

- return res
+ throw new Error(res.text() || "fallback message", { cause: res })

throw res could also be used, but wrapping with Error might be better for debugging and handling errors.

The benefits are that if not caught, the error can be handled by global error handlers and there's no need to check that the returned value is a Response, and it's still possible to explicitly catch errors if required.