[BUG] Error handling for fetch doesn't behave as expected. Always returns `Error: The script will never generate a response.`

[asimpletune]

Is there an existing issue for this?

• [X] I have searched the existing issues

What version of workers-rs are you using?

0.0.17

What version of wrangler are you using?

3.1.2

Describe the bug

Currently, a panic!() and an Err are treated the same by workers-rs.

This makes sense in the case of panic!(), but If an Err is returned, the caller still receives an Error: The script will never generate a response.', wrapped in a JsError type, regardless of what kind of error is actually returned by the downstream service.

Perhaps it makes sense to do this for someone curl'ing a service, but it breaks expectations in the situation where there's a bound service. Result types are designed for error propagation, so the client should be able to match on the error type, and recover from the error. As it currently stands this isn't possible.

Steps To Reproduce

Setup

1. Make two worker-rs services: service-a, service-b, and make service-b bound to service-a (I.e. service-a is upstream from service-b)
2. In service-b match on the path, and return different worker error types, e.g.

use worker::*;

// service-b handler

#[event(fetch)]
async fn main(mut req: Request, env: Env, ctx: Context) -> Result<Response> {
    match req.path().as_str() {
        "/hello" => Response::ok("Hello"),
        "/panic" => {
            panic!("freak out")
        }
        "/bad-encoding" => Err(worker::Error::BadEncoding),
        "/body-used" => Err(worker::Error::BodyUsed),
        "/json" => Err(worker::Error::Json((
            String::from("json error msg followed by status"),
            12,
        ))),
        "/break" => Err(worker::Error::from(
            "this is an example error message: 'invalid utf-8: corrupt contents'",
        )),
        _ => do_stuff(req).await,
    }
}

3. In service-a match on the request path, and call service-b. Then match on the response from service-b and convert the information we sent in step 2 into http responses, so the client can see them. E.g.

use worker::*;

// service-a handler

#[event(fetch)]
async fn main(req: Request, env: Env, ctx: Context) -> Result<Response> {
    return match req.path().as_str() {
        "/test" => Response::ok("test"),
        _ => {
            let svc = env.service("v1_InboundMail").unwrap();
            let result = svc.fetch_request(req).await;
            match result {
                Ok(rep) => Ok(rep),
                Err(worker_error) => match worker_error {
                    Error::BadEncoding => Response::error("BadEncoding", 500),
                    Error::BodyUsed => Response::error("BodyUsed", 500),
                    Error::Json(json) => Response::error(format!("Json error: '{}'", json.0), 500),
                    Error::JsError(js_err) => {
                        Response::error(format!("JsError: '{}'", js_err), 500)
                    }
                    Error::Internal(js_val) => {
                        Response::error(format!("JsVal: '{:#?}'", js_val), 500)
                    }
                    Error::BindingError(str) => {
                        Response::error(format!("BindingError: '{}'", str), 500)
                    }
                    Error::RouteInsertError(insert_error) => {
                        Response::error(format!("RouteInsertError: '{}'", insert_error), 500)
                    }
                    Error::RouteNoDataError => Response::error("Bad encoding", 500),
                    Error::RustError(str) => Response::error(format!("RustError: '{}'", str), 500),
                    Error::SerdeJsonError(err) => {
                        Response::error(format!("SerdeJsonError: '{}'", err), 500)
                    }
                    _ => todo!(),
                },
            }
        }
    };
}

4. Then try calling different endpoints

Results

We expect there to be recoverable and unrecoverable errors. However all the error calls respond with

JsError: 'Error: The script will never generate a response.'

The benign paths like /test and /hello both work as expected and return a 200 response with either "test" or "Hello".

Impact

This is currently making it difficult to debug a different bug in io-context of the the workerd runtime, as currently all types of errors are being treated the same by worker-rs. It also forces all errors to be handled by the downstream service, but they may not have all the context that the caller does as to how recover. Wrapping all errors in Response types is the only thing that can be done, but then now the client can't use types and pattern matching in order to recover, but must use the error message which is less brittle and sort of defeats the whole point of using Result anyways.

Notes

In wrangler 2.x.y the wrangler dev environment would just hang if you panicked, there wasn't this same ubiquitous response of Error: The script will never generate a response.. I don't know if that's related and if the wrangler dev version packages the same runtime as what's used in the cloud, but it's an interesting clue.