gql-dart / ferry

Stream-based strongly typed GraphQL client for Dart
https://ferrygraphql.com/
MIT License
593 stars 113 forks source link

[Feature Request] Add `ContextEntries` to requests that can be retrieved in custom `gql` `Link` #575

Closed tpucci closed 5 months ago

tpucci commented 5 months ago

In order to create advanced Links that can execute custom logic per request, we need a way to extend request with ContextEntries.

Currently, generated code for a request is:

  @override
  Request get execRequest => Request(
    operation: operation,
    variables: vars.toJson(),
    // no passed context. Thus, the default is `context = const Context()`
  );

gql_exec has a method .withContextEntry on Operation that we could use.

I'd be happy to contribute if you think it is a good issue 🙂

(😅 Hope I did not miss something in the API that already allows us to create context entries) Reference: #556

knaeckeKami commented 5 months ago

yes, a contributiuon would be great.

In order to create advanced Links that can execute custom logic per request, we need a way to extend request with ContextEntries.

Links can access access and edit context entries already!

tpucci commented 5 months ago

Hello @knaeckeKami ! Alright, here is my plan. What do you think ?

  1. Extend OperationRequest with a withContextEntry<T extends ContextEntry>(T entry) method
  2. Modify codegen to implement withContextEntry and pass it to execRequest
knaeckeKami commented 5 months ago

One way would be to add a Context? getter to the abstract OperationRequest class and adapt ferry_codegen to add this to the generated Request classes.

This would already allow users to add the context and modify Contexts via the built_value builders;

withContextEntry() is probably not strictly necessary since the built value classes already support that in their rebuild() methods, but it would allow to work with the context in TypedLinks where the exact type is not known.

A challenge is the serialization of the OperationRequest; Since the context is not serializable (it's a map of <Type, ContextEntry>, we would have to exclude it like updateResult.

tpucci commented 5 months ago

Ok got it. So:

  1. Context? getter
  2. codegen & exclude from serialization
knaeckeKami commented 5 months ago

sounds good to me!

bawahakim commented 4 months ago

@tpucci This is a life saver, thank you! Would it be possible to also have a withContextEntry and updateContextEntry in the OperationRequest so we can also modify the context globally, but before any link? My use case is I need to add a unique transaction id for each request header (not the same as the requestId), and also want to pass the original stack trace of the caller to simplify debugging (stack trace in chains loses the original caller info it seems).

I understand it was asked not to include it because we already have the rebuild method in the build value classes, but my use case would be to modify the context in a wrapper method, so it can be applied to all request.

tpucci commented 4 months ago

Hey @bawahakim I am not sure I understand. withEntry and updateEntry exist on Context, you could use those methods 🙂

If you want to set your context per request:

client.request(GSomeOperation((b) {
        b
          ..context = const Context().withEntry(YourEntry());
      })

If you want to set your context before all your existing links, create a link and put it first in your client link chain:

class YourLink extends Link {
  @override
  Stream<Response> request(Request request, [NextLink? forward]) {
    return forward(request.withContextEntry(YourEntry()));
  }
}

...

Client(
    link: Link.from([
          YourLink(),
          OtherLink(),
          HttpLink(
            const String.fromEnvironment('GRAPHQL_ENDPOINT'),
            httpClient: httpClient,
          )
    ]),
)

What do you think?

bawahakim commented 4 months ago

@tpucci Thank you! Let me explain a bit more my use case. The main issue I have is logging. Handling any error within a chain does not give me a clear stack track, only the trace of the links, so I lose information about the original caller. What I do is I inject the current stack trace in a wrapper method to ensure I can always know who the original calls was. I then use an extension method to handle any errors and use the original stack trace for my logger

class MyGqlClient {
  final Client client;

    Stream<OperationResponse<TData, TVars>> request<TData, TVars>(
    OperationRequest<TData, TVars> request,
  ) =>
      client.request(request).catchErrors(StackTrace.current);
}

extension RequestExtension<TData, TVars>
    on Stream<OperationResponse<TData, TVars>> {
  Stream<OperationResponse<TData, TVars>> catchErrors(
    StackTrace stackTrace,
  ) =>
      map((response) { 
        // Error handling
      }
}

class SomeConsumerClass {
  void someMethod() {
    MyGqlClient().request(GMyRequest());
  }
}

This way, my stack trace bypasses anything related to the links and I immediately get the stack from the original caller, making debugging easier. This works so far.

Now I also need to send a unique x-transaction-id header with every request. That's easy enough to do, but I also need to pass it down my catchErrors method so I can attach it to the logger (so I can match the client request error to the backend error). Note that I cannot use requestId because it serves a different purpose.

I can see 3 solutions, for which I have not found a way to implement:

  1. Continue with my approach, and inject the x-transaction-id header in a custom Link. However, I have no idea how to retrieve it later when I get an error in the catchErrors method
  2. Inject all I need in the request.context object wrapper method (stack trace, transaction id, etc). This way I can handle all error handling within the link without catchErrors
  3. Find another way to get the proper stack trace within the links without injecting it. Perhaps we could also do this at the library level, where we add the original stack trace to the errors

If the abstract OperationRequest had a withContextEntry method, then I could create a new request with my context entry, enabling solution 2.

Appreciate your help!

tpucci commented 4 months ago

Ha, got it 👍 Yeah you are right, there is no API to implement this behavior. Maybe we could create a transformContext like the existing transformOperation on OperationRequest. What do you think @knaeckeKami ?