How should Apollo plugins hook into the end of a request?

Help
1

Apollo’s graphql-extensions package enables requestDidStart to return an end hook that is called at the very end of the request lifecycle. It seems that with plugins, there is no explicit way to hook into this part of the lifecycle.

My assumption from reading the code is that this may be an intentional decision to remove redundancy between the request end hook and the willSendResponse hook. Is that correct?

I’m migrating an extension that times the request from start to finish. There are a number of legacy errors in the request pipeline code that will not trigger willSendResponse, so I wanted to double check.

When turning an extension into a plugin, should I replace my extension’s request end hook with a willSendResponse hook? Or is there a more applicable lifecycle event in the works?

2

We have End hooks available and specifically the executionDidStart hook has a handler that can alternatively return an object containing an executionDidEnd function. You can read more about it in our docs:

Let me know if that is what you’re looking for!

3

Thanks for the response, Michael. I am specifically asking about the end hook for requestDidStart, which was something provided in graphql-extensions but not in the new Apollo plugins.

In the request pipeline, an end hook can be pulled from the return value of an extension’s requestDidStart:

This end hook is called after the request pipeline ends, including when an error is thrown: apollo-server/requestPipeline.ts at 087862f0ab70860f10600b6aec419a8d43e0937c · apollographql/apollo-server · GitHub

Unfortunately, there does not seem to be an exact equivalent for plugins. Certain errors will not result in a call to willSendResponse on plugins. As a result, we don’t have a way to consistently listen for and respond to a request succeeding or failing without this lifecycle event also being exposed in a plugin. Is an equivalent plugin hook on Apollo’s roadmap?

4

Just checking back on this. Should I open an issue on the GitHub repo to request this?

5

@noam_c The requestDidEnd in graphql-extensions was misleading, because it wasn’t actually being invoked at the end of the request, rather than at the time that the response was about to be sent (which is notably different than “request did end”). Therefore, we opted to make the new request pipeline plugins more explicit about what the hook was and when it happened, and defer the implementation of the more precise requestDidEnd to future update. The constraint on building this the right way is currently a limitation in the way that we handle HTTP transport-specific behavior in Apollo Server, which we intend on improving in a future version of Apollo Server.

You can replicate the existing behavior by implementing both a willSendResponse and didEncounterErrors (the latter is necessary because due to a different constraint that is also related to the HTTP transport mechanism we have in place today that sometimes errors before the requestDidStart hook is invoked).

Please do open a feature request on the Apollo Server repository for this hook, but I wanted to provide you a bit of clarity as to the story here! Thanks for bringing this up, and I hope this helps!

6

@abernix Thank you so much for the clarity here!

I think what is challenging here is that didEncounterErrors may or may not be accompanied by willSendResponse. Is there a way to tell at the didEncounterErrors call whether or not willSendResponse will be called? We want to avoid double-counting the end of a request. Perhaps the real feature request here is for the error to have a property that indicates what will happen, so we can avoid brittle workarounds like checking error strings against a hardcoded list.

7

I’m pretty sure the concern here, while legitimate and a reason why we want to improve this API by actually offering a requestDidEnd, was also a constraint that existed before with graphql-extensions.

As a short-term work-around, I do think it’s plausible to attach a property to the error that you could use — I think that sounds like a viable suggestion. In that case, I would probably encourage you to open two issues:

  1. A feature request for requestDidEnd; and
  2. A request that asks for an attribute to be attached to internal errors which we know are going to circumvent willSendResponse which you could leverage in the short-term as a work-around. (With the explicit intention of this being deprecated by the completion of the former.) Honestly, this even sounds like it could graduate to a pull-requests-encouraged label pretty quickly and might be a straightforward contribution if you were so inclined!

Thoughts?