Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questions/Feedback after using this crate for Cloudflare Internship Hiring Assignment #78

Closed
abhishekc-sharma opened this issue Nov 1, 2021 · 2 comments

Comments

@abhishekc-sharma
Copy link

abhishekc-sharma commented Nov 1, 2021

Hi,
In the last week I applied to and attempted the Cloudflare Internship Hiring Assignment https://apply.cloudflareworkers.com/ with this crate. I would say I completed a majority of it but at some point I was too unsatisfied with how my code looked so I switched to using JavaScript for my worker. I had some general questions about what the expected/idiomatic way of doing certain things were for which I couldn't find any documentation/examples for -

  • What would be the idiomatic way to implement Error Handling in my worker ? My understanding of how it is setup currently is that any error from my handler would be converted to a worker::Error which by default causes an error response of some sort with an HTML body. If I were developing a REST API with JSON responses what would be the cleanest way to convert my errors to the appropriate JSON responses with a unique error code ?
  • Maybe related to the above - there doesn't seem to be any support for adding middleware to my Router. This proved troublesome when I had to add CORS headers to all my responses or to selectively authenticate some of my handlers. Is the expectation to use one of the existing crates in the ecosystem to handle this ? Or is it just something thats on the roadmap.

I hope my message doesn't come of as too aggressive. I realize that Rust support is in very early stages and would be happy to contribute to this crates code / documentation / examples etc.

@abhishekc-sharma abhishekc-sharma changed the title Idiomatic mechanism for Error handling Questions/Feedback after using this crate for Cloudflare Internship Hiring Assignment Nov 1, 2021
@nilslice
Copy link
Contributor

nilslice commented Nov 5, 2021

Hi @abhishekc-sharma -

Thanks for the thoughtful note, and for applying to Cloudflare's internship program! One of our interns @leoorshansky helped a TON on this project, maybe you can too :)

What would be the idiomatic way to implement Error Handling in my worker ? My understanding of how it is setup currently is that any error from my handler would be converted to a worker::Error which by default causes an error response of some sort with an HTML body. If I were developing a REST API with JSON responses what would be the cleanest way to convert my errors to the appropriate JSON responses with a unique error code ?

We should probably re-visit the decision to use Result types around the response returned from each handler. This was done to make some of the code more useful when writing the #[event] macros, but maybe time to take another look.

To answer your question though, I would write a utility function that returns Result<Response> and takes some message for you to re-use. Maybe to be mapped onto existing Result<Response> returned from places you might call Response::from_json etc.

I would be happy to consider any suggestions you have here! It's hard to cover everything when designing a framework like this -- and knowing when not to add features is just as important. We want to be generally useful, without imposing too many patterns on users who have varying use-cases.

Maybe related to the above - there doesn't seem to be any support for adding middleware to my Router. This proved troublesome when I had to add CORS headers to all my responses or to selectively authenticate some of my handlers. Is the expectation to use one of the existing crates in the ecosystem to handle this ? Or is it just something thats on the roadmap.

Yea, this is a great point. It's definitely something we should include support for. If you have any ideas on how you'd like to use it, it would be helpful to start with some design ideas.

I would think that this would likely need to be done at the Router level before you register any handlers, a la

let mut router = Router::new();
router.add(check_auth);
router.add(cors);
...

where check_auth and cors are fn pointers that are called with the same Request and Env arguments and return Result<Response> or Response directly.

Please feel free to leave notes / ideas here, and join us on Discord: https://discord.com/invite/cloudflaredev in the #workers-rs channel.

@abhishekc-sharma
Copy link
Author

Thanks a lot for you detailed response @nilslice .
I agree, it would be hard to specialize the library for every possible potential use case. Having to write my own wrapper is probably fair enough here.

Yeah, I was thinking of a router API something along those lines. I'm most familiar with the Router interface of the Go library Gorilla. Have to explore further to see how the other backend frameworks in Rust do the same thing.

@cloudflare cloudflare locked and limited conversation to collaborators Nov 29, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants