Improve stack-related compiler error messages#331
Improve stack-related compiler error messages#331olix0r merged 8 commits intover/profile-router-no-builderfrom
Conversation
Service stacks were built from "top" to bottom, so that the concrete service type is satisied after all middlewares are composed toegether. This leads to exceedingly complex compiler error messages. This change adds a `svc::Stack` builder that aids composing layers around a concrete inner service so that the compiler can more clearly identity the layer that violates the type contract. Note: this does *not* appear to improve compile times. No functional changes are intended.
olix0r
force-pushed
the
ver/stack-invesion
branch
from
b59db2f to
546c72e
Compare
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
I'm very +1 on this for the improved error messages alone. It's a shame that the build times aren't improved, but that's not too surprising IMO — the build time improvement from doing this with our pre-tower-layer stack types was probably an artifact of that specific implementation.
Have we considered upstreaming this after proving it out in the proxy?
The builder type should only be used when composing layers. Now that we have `Builder::into_inner`, we need not store a builder in the profile router.
|
@hawkw i'd encourage tower to mirror these changes. from the linkerd perspective, i'm not particularly bothered by maintaining our own builder types, especially if we continue ensure they are only used narrowly (i.e. code doesn't depend on concrete builder types). |
👍 @LucioFranco may be interested in this! |
|
@olix0r by chance, do you have an example of error messages and how they differ? |
kleimkuhler
left a comment
kleimkuhler
left a comment
There was a problem hiding this comment.
Overall change looks good. An example could be helpful, but otherwise I just left a few questions and small changes.
|
@LucioFranco sorry, it's pretty annoying (time consuming) to generate these errors, so I'll have to leave that as an exercise to others. The problem is actually pretty easy to understand, though: If I construct a stack as |
|
@olix0r I'm onboard with that! I remember the discussion we had a while back. I do think with tower 0.1 we should keep it how it is now. Does linkerd have any plans around updating to futures 0.3? I guess there should be some talk around that...in relation to tower. |
4 participants
Service stacks were built from "top" to bottom, so that the concrete
service type is satisied after all middlewares are composed toegether.
This leads to exceedingly complex compiler error messages.
This change adds a
svc::Stackbuilder that aids composing layersaround a concrete inner service so that the compiler can more clearly
identity the layer that violates the type contract.
Note: this does not appear to improve compile times.
No functional changes are intended.