From fabd9b35190c3a4ee2e0264e261fac191486769b Mon Sep 17 00:00:00 2001
From: Thomas Obenaus <obenaus.thomas@gmail.com>
Date: Tue, 10 Nov 2020 14:47:16 +0100
Subject: [PATCH] Fix: ensure that status codes are checked in any case

If this check is missing then responses from the server with the graphql end-point  that are not compliant to the graphql spec will be just ignored (not treated as an error).
One reason for this behavior of the server could be (e.g. springboot) that the request is already rejected before it reaches the graphql end-point due to missing authorization. In this case the security filter that does not know about graphql responds with an error that is not compliant to the GQL spec.
The same situation would be in case of a 404, 403 etc.
---
 graphql.go | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/graphql.go b/graphql.go
index 05c29b7..32253ac 100644
--- a/graphql.go
+++ b/graphql.go
@@ -137,6 +137,8 @@ func (c *Client) runWithJSON(ctx context.Context, req *Request, resp interface{}
 		return errors.Wrap(err, "reading body")
 	}
 	c.logf("<< %s", buf.String())
+	bodyStr := buf.String()
+
 	if err := json.NewDecoder(&buf).Decode(&gr); err != nil {
 		if res.StatusCode != http.StatusOK {
 			return fmt.Errorf("graphql: server returned a non-200 status code: %v", res.StatusCode)
@@ -147,6 +149,17 @@ func (c *Client) runWithJSON(ctx context.Context, req *Request, resp interface{}
 		// return first error
 		return gr.Errors[0]
 	}
+
+	// Handle the http status codes before handling response from the graphql endpoint.
+	// If this is not done then !200 status codes will just be ignored without the caller even noticing, instead
+	// the caller just gets back an empty result set, suggesting that the query did not found any result.
+	// The reason for this is that for example in case of a 404,401,403 etc. the request is rejected before
+	// it even hits an graphql handler on the server side.
+	// As a result the response returned by this non graphql component is not compliant to the graphql spec.
+	if res.StatusCode != http.StatusOK {
+		return fmt.Errorf("graphql: server returned a non-200 status code: %v - %v", res.StatusCode, bodyStr)
+	}
+
 	return nil
 }
 
@@ -208,6 +221,8 @@ func (c *Client) runWithPostFields(ctx context.Context, req *Request, resp inter
 		return errors.Wrap(err, "reading body")
 	}
 	c.logf("<< %s", buf.String())
+	bodyStr := buf.String()
+
 	if err := json.NewDecoder(&buf).Decode(&gr); err != nil {
 		if res.StatusCode != http.StatusOK {
 			return fmt.Errorf("graphql: server returned a non-200 status code: %v", res.StatusCode)
@@ -218,6 +233,16 @@ func (c *Client) runWithPostFields(ctx context.Context, req *Request, resp inter
 		// return first error
 		return gr.Errors[0]
 	}
+
+	// Handle the http status codes before handling response from the graphql endpoint.
+	// If this is not done then !200 status codes will just be ignored without the caller even noticing, instead
+	// the caller just gets back an empty result set, suggesting that the query did not found any result.
+	// The reason for this is that for example in case of a 404,401,403 etc. the request is rejected before
+	// it even hits an graphql handler on the server side.
+	// As a result the response returned by this non graphql component is not compliant to the graphql spec.
+	if res.StatusCode != http.StatusOK {
+		return fmt.Errorf("graphql: server returned a non-200 status code: %v - %v", res.StatusCode, bodyStr)
+	}
 	return nil
 }