cloudflare / cloudflare/workers-rs
[BUG] Error handling for fetch doesn't behave as expected. Always returns `Error: The script will never generate a response.`
- 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
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