Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
tpucci opened this issue Feb 2, 2024 · 9 comments

Comments

@tpucci
Copy link
Contributor

tpucci commented Feb 2, 2024

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
Copy link
Collaborator

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
Copy link
Contributor Author

tpucci commented Feb 2, 2024

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
Copy link
Collaborator

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
Copy link
Contributor Author

tpucci commented Feb 2, 2024

Ok got it. So:

  1. Context? getter
  2. codegen & exclude from serialization

@knaeckeKami
Copy link
Collaborator

sounds good to me!

@bawahakim
Copy link

bawahakim commented Mar 7, 2024

@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
Copy link
Contributor Author

tpucci commented Mar 7, 2024

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
Copy link

bawahakim commented Mar 8, 2024

@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
Copy link
Contributor Author

tpucci commented Mar 8, 2024

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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants