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

Ability to print query string before POSTing #36

Open
mirosval opened this issue May 28, 2018 · 7 comments
Open

Ability to print query string before POSTing #36

mirosval opened this issue May 28, 2018 · 7 comments
Labels
API decision NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. thinking

Comments

@mirosval
Copy link

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

dmitshur commented May 31, 2018

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.

@dmitshur dmitshur added API decision thinking NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 31, 2018
@andredurao
Copy link

andredurao commented Jun 13, 2018

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

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

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

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

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

flexzuu commented Aug 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API decision NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. thinking
Development

No branches or pull requests

5 participants