shurcooL / githubv4

Package githubv4 is a client library for accessing GitHub GraphQL API v4 (https://docs.github.com/en/graphql).
MIT License
1.11k stars 89 forks source link

Ability to print query string before POSTing #36

Open mirosval opened 6 years ago

mirosval commented 6 years ago

Currently the whole process happens in the do function of the Client and there is no easy way of printing out the query string.

I would like to inspect this query string during the development of my app, but it would be useful for other things, such as logging.

dmitshur commented 6 years ago

I will need some time to think about how this can be implemented in a simple and clean way, if at all, because it's not immediately clear to me.

I know people prefer to use different solutions for logging: some want to use the standard log package, others have custom loggers. I want to defer dealing with that, since there's no one great solution. Ideally, it's something that can be done outside this package.

In the meantime, for local development of your app, I suggest you simply temporarily modify the source code of do to add a fmt.Printf statement. That's what I do.

For logging, for now, you can factor it outside of githubv4 with something like this:

type loggingClient struct {
    *githubv4.Client
}

func (c *loggingClient) Query(ctx context.Context, q interface{}, variables map[string]interface{}) error {
    log.Println(...)
    // if you want to print out the query, you can copy the constructQuery() code from graphql library
    return c.Client.Query(ctx, q, variables)
}

That is a short term solution you can start using right away. We can use this issue to track this problem and any developments.

andredurao commented 6 years ago

I've tried to use the constructQuery function, but it's not exported... So when I used it occurs this error: cannot refer to unexported name graphql.constructQuery

func constructQuery(v interface{}, variables map[string]interface{}) string {
        query := query(v)
        if variables != nil {
                query = "query(" + queryArguments(variables) + ")" + query
        }
        println(query) // Use the default logger here
        return query
}

This issue maybe under https://github.com/shurcooL/graphql instead of here; I'll try to open a PR when possible 👍

dmitshur commented 6 years ago

you can copy the constructQuery() code from graphql library

The keyword is copy. It's unexported because it's low-level internal functionality and not a part of the public graphql API. You'll need to copy it into your code in order to use it there.

NathanZook commented 5 years ago

I'm new to go, but I wondering about just wrapping the HTTP client with a logger. This runs into problems, and I think I know why: compare NewClient here & in graphql to the one in OAuth: https://github.com/golang/oauth2/blob/master/oauth2.go#L321. In particular, OAuth returns an http client with a wrapped Transport. http.Client#Do eventually calls transport.RoundTrip. https://github.com/golang/oauth2/blob/master/oauth2.go#L321 Certainly, this is very low-level stuff, but it allows OAuth.NewClient to feed your code, which only accepts http.Client. If graphql followed the methodology of OAuth, I would expect that a logger could similarly be be inserted into the chain, making this request simply not a proper concern of the package.

NathanZook commented 5 years ago

@mirosval @andredurao I've chased the rabbit hole a bit. Especially if you want a general solution, you want an http.Client where the Transport does the logging in RoundTrip (https://golang.org/src/net/http/client.go#L250). Keep in mind that http.Request.Body is only an io.ReadCloser, so you will probably want to use io.Copy (https://golang.org/src/io/io.go?s=12784:12844#L353) to copy the body into the log string. When you do so, however, you will consume Body before it is sent--you will want to replace the Request.Body with a source that can again be copied.

NathanZook commented 5 years ago

Request.getBody is what is needed. This is used for 307 & 308 responses, as documented in Request.NewRequest (https://golang.org/src/net/http/request.go#L792)

flexzuu commented 5 years ago

this is really helpful: https://gist.github.com/flexzuu/95cfc042d27a152c64ccdca0a36c75d8