workers-rs icon indicating copy to clipboard operation
workers-rs copied to clipboard

API Improvement ideas

Open ByteAlex opened this issue 4 years ago • 0 comments

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.

  1. 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)

  1. 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 () {}

ByteAlex avatar Jan 08 '22 14:01 ByteAlex