[3/n] [dropshot-endpoint] factor out endpoint and parameter validation #1008
Conversation
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
|
|
||
| error: `self` parameter is only allowed in associated functions | ||
| --> tests/fail/bad_endpoint2.rs:13:23 | ||
| | | ||
| 13 | async fn bad_endpoint(self) -> Result<HttpResponseOk<()>, HttpError> { | ||
| | ^^^^ not semantically valid as function parameter | ||
| | | ||
| = note: associated functions are those in `impl` or `trait` definitions | ||
|
|
||
| error[E0401]: can't use generic parameters from outer item | ||
| --> tests/fail/bad_endpoint2.rs:13:23 | ||
| | | ||
| 9 | #[endpoint { | ||
| | ---------- `Self` type implicitly declared here, by this `impl` | ||
| ... | ||
| 13 | async fn bad_endpoint(self) -> Result<HttpResponseOk<()>, HttpError> { | ||
| | ^^^^ | ||
| | | | ||
| | use of generic parameter from outer item | ||
| | refer to the type directly here instead |
There was a problem hiding this comment.
I think these errors in particular are unhelpful, and detecting them syntactically is much cleaner.
There was a problem hiding this comment.
What does "detecting them syntactically" mean and how does that relate to this change? Agreed that these errors are not helpful.
There was a problem hiding this comment.
By "syntactically" I just mean looking at the token stream and rejecting code within the proc macro, rather than "semantically", generating code that will fail to compile and relying on rustc's errors. (Is there a better term for this? I've always used syntactically/semantically for this in the context of proc macros)
Created using spr 1.3.6-beta.1
|
(Looks like the tests aren't running because this is a dependent PR not targeting main. Yay for github's terrible support for stacks.) |
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
sunshowers
changed the title
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
dropshot_endpoint/src/error_store.rs
Outdated
| pub(crate) struct ErrorSink<'a, T> { | ||
| // It's a bit weird to use both a lifetime parameter and a RefCell, but it | ||
| // makes sense here. The lifetime parameter ensures that the error context | ||
| // doesn't outlive the collector, and the RefCell exists so we can |
There was a problem hiding this comment.
Ah, the collector is just the thing that collects and stores errors.
There was a problem hiding this comment.
Fixed this comment up.
sunshowers
changed the base branch from
sunshowers/spr/main.dropshot-endpoint-factor-out-endpoint-and-parameter-validation
to
main
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
|
|
||
| use std::cell::RefCell; | ||
|
|
||
| /// Top-level struct that holds all errors encountered during the invocation of |
| /// So the typical Rust pattern of returning on the first error isn't quite good | ||
| /// enough for us, and what we need is: | ||
| /// | ||
| /// * a hierarchical way to collect errors |
There was a problem hiding this comment.
it's not clear to my why hierarchy is important or how it manifests to users... but this is great, and no need to block on my confusion!
There was a problem hiding this comment.
Basically the hierarchy is:
top level
|-> metadata
|-> signature
|-> request context
|-> each extractor
|-> return type
and errors in some node in the tree should not stop sibling nodes from being validated.
sunshowers
deleted the
sunshowers/spr/dropshot-endpoint-factor-out-endpoint-and-parameter-validation
branch
We're going to reuse a lot of this code for server endpoints. Refactor code to
prepare for that. I think it's overall a nice improvement :)
For successful generation, there are absolutely no changes in output. (This is
a standalone PR, so it's easy to see this.)
In error cases:
Also add a little bit of error-handling infrastructure in the form of an
ErrorStore. This makes it reasonable to have this kind of nestedstructure, where each component can determine whether it produced errors. We're
going to use this more extensively within the server macro, since that has
to maintain separate error contexts per endpoint.
As part of this,
do_endpointanddo_channelno longer returnResulttypes -- all errors are handled internally, and instead they return a pair of output and a list of errors. This resolves an inconsistency that I've been bothered by for a bit.Depends on #1007.