WHOA... change of execution order... router_service.map_response is called PRIOR to the subgraph execution

[golfingal72]

I started working with 0.9.4 and found that the router_service.map_response is called PRIOR to the subgraph execution... this is NOT GOOD... the map_response should be called after the downstream services have executed because the downstream services may be sending things back (headers) that the FINAL response needs to send back to the caller

how to get around this??? I need the subgraphs to execute first

I am using map_response to track overall response times and if you run it prior to subgraph execution then it doesn't work at all

[golfingal72]

ping o0Ignition0o

[golfingal72]

does the order of the services matter in the serviceBuilder??

i have .map_request
.checkpoint
.buffer(20_000)
.service(service)
.map_response(|mut resp| {
.boxed()

I'll try switching around

[golfingal72]

nope, that doesn't compile

[o0Ignition0o]

Hey!

FYI The team is mostly online during CET working hours.

I've updated the plugin example I use to test the APIs to gather and display subgraph response times.

It's worth noting that given the Body of the response is now a stream, we need to wait until we receive the first response before we have a look at the context.

Please let me know if it helps!

[golfingal72]

o0Ignition0o

Thanks for the example...

The execution order has changed, why???? I don't believe it's the right thing... Your documentation chart on execution order doesn't reflect what is going on now.... https://www.apollographql.com/docs/router/customizations/overview/
The subgraph example you give is making the actual user have to force the future to run... this doesn't follow the simple models that you lay out for people to follow which were easy to understand to introduce custom logic and worked really nicely

IMHO I think there is a flaw in the changes that have been introduced into the infrastructure because now there is "magic" that people have to implement if they want the execution order to happen as they think it would... I ended up writing 2 plugins, but if people write multiple plugins that are mapping the execution, and it's not happening in the order they expect, you are going to have to continually deal with support issues and complaints.

It seems that at the level where you interpret the custom plugins for execution that you can layer them in so that the execution order can be retained???

[o0Ignition0o]

The execution order has not changed. However the async nature of response payloads introduces some complexity which we are currently addressing.

That being said if updating your plugins is too much of a change for now, I suggest you use router v0.9.3 until we provide higher level primitives to work with the Execution/Router response body.

These will come with migration / release notes on how to migrate, and the migration process will be a more pleasant experience.

[o0Ignition0o]

#1260 once merged should ease this by a substantial amount!

[Geal]

You're right that this behaviour is surprising, so #1260 should get back the old behaviour. That said, adding support for @defer will expose complexity that we cannot avoid, and that will feel magic or imprecise. We probably won't be able to guarantee the order of deferred responses (we might be able to keep the main response as first element of the stream), it's impossible to know in advance how many responses will appear in the stream, and even detecting if we reached the last response will be complex.
We will try and make the main experience (one request, no @defer) straightforward, but getting it right will need a few adjustments.

As a side note, for specific problems like that, you should open an issue instead of another discussion, this will be easier to track. And please don't ping @o0Ignition0o directly to get faster answers, he'll soon go on much deserved vacations so he won't respond 😅

[golfingal72]

Thank you both! o0Ignition0o have a great vacation...

[golfingal72]

o0Ignition0o

so i got access to the response after subgraph execution... using

          // we can now transform the router_response!
            resp
              .map(|response| {
                 // we need to clone the context in order to use it in the closure
                 let context = context.clone();
                 log::info!(target: "app::debug", "IN MAIN ROUTER");
                
                 response.map(move |body| handle_router_response(body, &context))
            })
            .boxed()

HOW do i get access to the response.status and the response.headers_mut after the subgraph execution? I possibly may need to add response headers and i need access to the overall response status?

QQ: there are 2 clones of the context.. is it actually copying the whole object twice or is it just a reference to the object?

[golfingal72]

Will the change you made on your PR answer this question as well?

[Geal]

the PR guarantees that the subgraph execution for the response happens before the map_response calls in execution plugins and router plugins. In those methods you will be able to access and edit the response headers. If you need to share data between the different steps, you can pput that in the context object. Cloning it is cheap, it is in an Arc

[golfingal72]

Fantastic... do you have an ETA on when you will do your next tagged release?