cloudflare / cloudflare/workers-rs

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

Open
#352 0 comments 4 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
3.7k
Forks
429
Avg merge
20h 28m
Merged PRs (30d)
7

Description

### 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.

```rust
use worker::*;

// service-b handler

#[event(fetch)]
async fn main(mut req: Request, env: Env, ctx: Context) -> Result {
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.

```rust
use worker::*;

// service-a handler

#[event(fetch)]
async fn main(req: Request, env: Env, ctx: Context) -> Result {
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.

Contributor guide

Open the contributing guide

Research direction

Start with the service-binding reproduction and the fetch_request call in service-a, then trace how the downstream Result error becomes a JsError. Reproduce the /panic, /bad-encoding, /body-used, /json, and /break paths; done means panic behavior remains distinct while recoverable errors preserve enough information for service-a to match and handle them.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, cloud
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.