feat(gateway): Provide context to didEncounterError for RGDS
#600
Conversation
| @@ -192,7 +192,8 @@ export class RemoteGraphQLDataSource<TContext extends Record<string, any> = Reco | |||
| public didEncounterError( | |||
| error: Error, | |||
| _fetchRequest: Request, | |||
| _fetchResponse?: Response | |||
| _fetchResponse?: Response, | |||
| _context?: TContext, | |||
There was a problem hiding this comment.
It's a shame that this function doesn't take options (named) arguments but given that it doesn't, this seems like the best change.
There was a problem hiding this comment.
Agree - I think it would be nice to move to an options object in a breaking change in the future. Should I open an issue?
There was a problem hiding this comment.
🤷 there are deeper issues with this API (like how the class name sounds like the not really related DataSource and how the method names sound like the AS plugin API) that might be worth addressing at some point
| @@ -1,5 +1,5 @@ | |||
| import { fetch } from '../../__mocks__/apollo-server-env'; | |||
| import { makeFetchHappenFetcher} from '../../__mocks__/make-fetch-happen-fetcher'; | |||
| import { makeFetchHappenFetcher } from '../../__mocks__/make-fetch-happen-fetcher'; | |||
There was a problem hiding this comment.
There are those who would complain about applying prettier to a file, but not me.
There was a problem hiding this comment.
This is actually a result of using inline snapshots (which has the effect of prettifying the file when a new snap is printed). I ended up not using a snapshot but the changes persisted. I don't mind either - I'd usually isolate to a commit but hopefully this one was small enough to not be a bother.
8c366e2 to
2bfea5f
Compare
This PR introduces a fourth (optional) argument
contextto theRemoteGraphQLDataSourcesdidEncounterErrorhook. This is a fairly small, non-breaking change to the API here and it seems fairly reasonable to want access tocontextfrom within this hook.