Add protocol specific routers #1666
Conversation
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| @@ -36,3 +36,7 @@ use crate::body::BoxBody; | |||
|
|
|||
| #[doc(hidden)] | |||
| pub type Response<T = BoxBody> = http::Response<T>; | |||
|
|
|||
| pub trait IntoResponse<Protocol> { | |||
There was a problem hiding this comment.
Add #[doc(hidden)]? Since only codegen needs to be aware of this trait.
There was a problem hiding this comment.
There's already a #[doc(hidden)] at the module level in lib.rs. The Response above is double hidden, which threw me off too - should I remove that?
There was a problem hiding this comment.
There's already a #[doc(hidden)] at the module level in lib.rs
Ah, missed that. Thought it wasn't because Response was doc hidden.
Ok then, no need to double hide Response --- although, I'm thinking that middleware authors will need access to it right?
There was a problem hiding this comment.
I think that third-parties might want to see IntoResponse and FromRequest eventually? They'll need it to implement their own protocols etc. I was thinking of keeping them hidden until everything in the RFC has been implemented. I'm not 100% sure on whether IntoResponse needs a second parameterization over the operation, which would be a breaking change.
I don't mind (double?) unhiding Response, no strong feelings about that.
rust-runtime/aws-smithy-http-server/src/routing/routers/aws_json.rs
Outdated
Show resolved
Hide resolved
rust-runtime/aws-smithy-http-server/src/routing/routers/rest.rs
Outdated
Show resolved
Hide resolved
…smithy-rs into harryb/protocol-specific-router
| fn into_response(self) -> http::Response<BoxBody> { | ||
| match self { | ||
| Error::MethodDisallowed => super::method_disallowed(), | ||
| _ => http::Response::builder() |
There was a problem hiding this comment.
We have the opportunity here to provide better error messages if we wanted. For example, if there is a missing header should we status code 400?
There was a problem hiding this comment.
We should only do things that are in the spec of these protocols. I don't even know if implementations of these protocols return 405 when the URI matches the route but the verb doesn't... These are good questions to be asking in the awslabs/smithy issue tracker, and push for better protocol test coverage.
There was a problem hiding this comment.
| @@ -25,8 +25,6 @@ use crate::{protocols::Protocol, response::Response}; | |||
|
|
|||
| #[derive(Debug)] | |||
| pub enum RuntimeErrorKind { | |||
| /// The requested operation does not exist. | |||
There was a problem hiding this comment.
This a good example of the general approach laid out in the Protocol specific Errors section of the RFC.
- Make a protocol specific error
- Implement
IntoResponse<Protocol>for it - Remove the variant from
RuntimeErrorKind
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation and Context
Routertype? #1606Description
IntoResponse<Protocol>to allow for multiple implementations ofIntoResponse.UnknownOperationfromRuntimeError.http::ResponseviaIntoResponse.Routerto use protocol specific routers internally.