API Improvement ideas
Hello,
I've been working with the workers-rs api recently and noticed it has got a few rough edges in my opinion.
I've thought how that could be improved thus I am suggesting this here, willing to create a PR once the idea has been approved.
- Responses
- The constructors of responses return a Result, which forces the user to call map to access response functions.
=> Should be the Self-Object not conatained in a Result, user should call Ok(Response) later. I am aware of the error-possibility from set_headers, those can be wrapped into an Response-internal error object which is unwrapped in the final error handling. (See next point)
- Error handling using ? operator
- Currently the router takes a function with a worker::Result<Response>, and everything which is not Ok() will be rendered as error with panic. (CF error page)
- It's cool, that the worker::Error type is a thiserror:Error derive and can be constructed using the From trait from basically every possible Rest error, but it's still missing flexibility.
=>Make an error handling function built into the router which a) supports any generic error type with a "FromWorkerError" trait b) has the signature set_error_handler<T: FromWorkerError, E: 'static + Copy + Fn(T) -> Response>(fun: E). This function will unwrap any possible error into a custom error message, if desired. The default error handler can just be a panic!().
For reference: My current "workaround" code
pub struct ResponseWrapper {
body: Option<Vec<u8>>,
headers: worker::Headers,
status: u16,
error: Option<worker::Error>,
}
impl ResponseWrapper {
pub fn json<T: Serialize>(data: &T) -> Result<Self, serde_json::Error> {
let data = serde_json::to_vec(data)?;
Ok(Self::bytes(data)
.append_header("content-type", "application/json"))
}
pub fn bytes(data: Vec<u8>) -> Self {
Self {
body: Some(data),
headers: worker::Headers::new(),
status: 200,
error: None,
}
}
pub fn text<S: Into<String>>(data: S) -> Self {
Self {
body: Some(data.into().into_bytes()),
headers: worker::Headers::new(),
status: 200,
error: None,
}
}
pub fn empty() -> Self {
Self {
body: None,
headers: worker::Headers::new(),
status: 204,
error: None,
}
}
pub fn with_status(mut self, status: u16) -> Self {
self.status = status;
self
}
pub fn append_header(mut self, key: &str, val: &str) -> Self {
if let Some(err) = self.headers.append(key, val).err() {
self.error = Some(err);
}
self
}
}
impl Into<worker::Result<worker::Response>> for ResponseWrapper {
fn into(self) -> worker::Result<Response> {
if let Some(err) = self.error {
Err(err)
} else {
let headers = self.headers;
let status = self.status;
if let Some(data) = self.body {
Response::from_body(ResponseBody::Body(data))
.map(|resp| resp.with_headers(headers))
.map(|resp| resp.with_status(status))
} else {
Response::from_body(ResponseBody::Empty)
.map(|resp| resp.with_headers(headers))
.map(|resp| resp.with_status(status))
}
}
}
}
pub struct RequestWrapper<T, D> {
func: fn(worker::Request, worker::RouteContext<D>) -> T,
}
pub type ToResponseFn<'a, D> = Rc<dyn Fn(worker::Request, worker::RouteContext<D>) -> LocalBoxFuture<'a, ResponseWrapper>>;
pub trait ToResponseJson<'a, R, D, E> {
fn to_response_json(self, err: E) -> ToResponseFn<'a, D>;
}
pub trait ToResponsePlain<'a, R, D, E> {
fn to_response_plain(self, err: E) -> ToResponseFn<'a, D>;
}
pub trait ToResponseEmpty<'a, R, D, E> {
fn to_response_empty(self, err: E) -> ToResponseFn<'a, D>;
}
impl<R: 'static, T: 'static + Future<Output=Result<R, crate::model::Error>>, D: 'static> RequestWrapper<T, D> {
pub fn from(func: fn(worker::Request, worker::RouteContext<D>) -> T) -> Self {
Self {
func,
}
}
}
impl<
'a,
R: 'static + Serialize,
T: 'static + Future<Output=Result<R, crate::model::Error>>,
D: 'static,
E: 'static + Copy + Fn(crate::model::Error) -> ResponseWrapper
> ToResponseJson<'a, R, D, E> for RequestWrapper<T, D> {
fn to_response_json(self, err: E) -> ToResponseFn<'a, D> {
Rc::new(move |req, ctx| Box::pin(to_response_json(req, ctx, err, self.func)))
}
}
impl<
'a,
R: 'static + Into<String>,
T: 'static + Future<Output=Result<R, crate::model::Error>>,
D: 'static,
E: 'static + Copy + Fn(crate::model::Error) -> ResponseWrapper
> ToResponsePlain<'a, R, D, E> for RequestWrapper<T, D> {
fn to_response_plain(self, err: E) -> ToResponseFn<'a, D> {
Rc::new(move |req, ctx| Box::pin(to_response_plain(req, ctx, err, self.func)))
}
}
impl<
'a,
R: 'static + None,
T: 'static + Future<Output=Result<R, crate::model::Error>>,
D: 'static,
E: 'static + Copy + Fn(crate::model::Error) -> ResponseWrapper
> ToResponseEmpty<'a, R, D, E> for RequestWrapper<T, D> {
fn to_response_empty(self, err: E) -> ToResponseFn<'a, D> {
Rc::new(move |req, ctx| Box::pin(to_response_empty(req, ctx, err, self.func)))
}
}
pub fn default_error_wrapper(err: crate::model::Error) -> ResponseWrapper {
ResponseWrapper::text(format!("Error: {}", err))
.with_status(500)
}
async fn to_response_json<
R: 'static + Serialize,
T: Future<Output=Result<R, crate::model::Error>>,
D,
E: Fn(crate::model::Error) -> ResponseWrapper
>(req: worker::Request, ctx: worker::RouteContext<D>, error_wrapper: E, func: fn(worker::Request, worker::RouteContext<D>) -> T) -> ResponseWrapper {
let result = func(req, ctx).await;
match result {
Ok(data) => {
match ResponseWrapper::json(&data) {
Ok(resp) => resp,
Err(err) => {
// todo: error_wrapper?
ResponseWrapper::text(format!("{}", err))
.with_status(500)
}
}
}
Err(err) => {
error_wrapper(err)
}
}
}
async fn to_response_plain<
R: 'static + Into<String>,
T: Future<Output=Result<R, crate::model::Error>>,
D,
E: Fn(crate::model::Error) -> ResponseWrapper
>(req: worker::Request, ctx: worker::RouteContext<D>, error_wrapper: E, func: fn(worker::Request, worker::RouteContext<D>) -> T) -> ResponseWrapper {
let result = func(req, ctx).await;
match result {
Ok(data) => ResponseWrapper::text(data),
Err(err) => {
error_wrapper(err)
}
}
}
async fn to_response_empty<
R: 'static + None,
T: Future<Output=Result<R, crate::model::Error>>,
D,
E: Fn(crate::model::Error) -> ResponseWrapper
>(req: worker::Request, ctx: worker::RouteContext<D>, error_wrapper: E, func: fn(worker::Request, worker::RouteContext<D>) -> T) -> ResponseWrapper {
let result = func(req, ctx).await;
match result {
Ok(_) => ResponseWrapper::empty(),
Err(err) => {
error_wrapper(err)
}
}
}
trait None {}
impl None for () {}