Return error list while validating schema.

[arijitAD]

Fixes GRAPHQL-376

---

This change is 

Docs Preview:

[pawanrawal]

Reviewable status: 0 of 4 files reviewed, 7 unresolved discussions (waiting on @arijitAD, @MichaelJCompton, and @pawanrawal)

---

graphql/schema/gqlschema.go, line 422 at r1 (raw file):

					continue
				}
				errs = append(errs, directiveValidators[dir.Name](schema, typ, field, dir, secrets)...)

We are not using appendIfNotNull anymore? Would that mean null errors are also appended now?

---

graphql/schema/gqlschema_test.yml, line 819 at r1 (raw file):

    {"message": "Type Query; Field getAuthor1: has 2 arguments for @custom directive, it should contain exactly 1 argument.",
     "locations":[{"line":7, "column":32}]},
    {"message" : "Type Query; Field getAuthor1; url field inside @custom directive is invalid.", "locations" : [{"line":7, "column":52}]}

nice

---

graphql/schema/gqlschema_test.yml, line 990 at r1 (raw file):

    {"message": "Type Query; Field getAuthor1; has parameters in url along with graphql field inside @custom directive, url can't contain parameters if graphql field is present.",
     "locations":[{"line":7, "column":32}]},
    {"message": "Type Query; Field getAuthor1; @custom directive, graphql must use fields with a variable definition, found `id`.",

I see this error in a bunch of places, we should probably fix the query in all these places so that it isn't reported. Then we can add a new test to show how multiple errors are reported when this error is still present.

---

graphql/schema/gqlschema_test.yml, line 992 at r1 (raw file):

    {"message": "Type Query; Field getAuthor1; @custom directive, graphql must use fields with a variable definition, found `id`.",
     "locations":[{"line":10, "column":15}]},
    {"message": "Type Query; Field getAuthor1: inside graphql in @custom directive, while json unmarshaling result from remote introspection query: invalid character '<' looking for beginning of value",

This error is coming from trying to make the introspection request on the given URL. Firstly, I think the error message itself should be fixed to say its a 404. Also, we should not be making the request if the URL itself isn't in the correct format or if the graphql query definition above failed validation.

---

graphql/schema/gqlschema_test.yml, line 1488 at r1 (raw file):

    },
    {
      "message": "Type Post; Field author; @custom directive, graphql must use fields with a variable definition, found ``.",

It says found ``, whereas it should be found $id. Can you check why that is the case?

---

graphql/schema/gqlschema_test.yml, line 2042 at r1 (raw file):

    },
    {
      "message": "Type Author; Field name; url path inside @custom directive uses a field bar that can be null.",

Nice

---

graphql/schema/rules.go, line 1548 at r1 (raw file):

			}
		}
		if err := validateRemoteGraphql(&remoteGraphqlMetadata{

If there are any errors until this point, then we should just return even before making the introspection calls. Some errors that I can think of are invalid url, graphql query being incorrect (not using variables and such).

[arijitAD]

Reviewable status: 0 of 5 files reviewed, 7 unresolved discussions (waiting on @arijitAD, @MichaelJCompton, and @pawanrawal)

---

graphql/schema/gqlschema.go, line 422 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

We are not using appendIfNotNull anymore? Would that mean null errors are also appended now?

Appending nil slice to a slice wouldn't change anything
https://play.golang.org/p/2aeIlhO84Bv

---

graphql/schema/gqlschema_test.yml, line 990 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

I see this error in a bunch of places, we should probably fix the query in all these places so that it isn't reported. Then we can add a new test to show how multiple errors are reported when this error is still present.

Done.

---

graphql/schema/gqlschema_test.yml, line 992 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

This error is coming from trying to make the introspection request on the given URL. Firstly, I think the error message itself should be fixed to say its a 404. Also, we should not be making the request if the URL itself isn't in the correct format or if the graphql query definition above failed validation.

Done.

---

graphql/schema/gqlschema_test.yml, line 1488 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

It says found ``, whereas it should be found $id. Can you check why that is the case?

It was happening because we didn't return earlier.

---

graphql/schema/rules.go, line 1548 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

If there are any errors until this point, then we should just return even before making the introspection calls. Some errors that I can think of are invalid url, graphql query being incorrect (not using variables and such).

Done.

[pawanrawal]

Mostly looks good. Please these tests and then this should be good to go.

1. A field has error in two directives, both of them should be returned.
2. Two fields each have one error corresponding to a directive, and both should be returned.
3. Two types have errors and both of them should be returned.

Reviewed 5 of 5 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @arijitAD, @MichaelJCompton, and @pawanrawal)

---

graphql/schema/remote.go, line 39 at r2 (raw file):

	if u.RawQuery != "" {
		return fmt.Errorf("POST method cannot have variables in url: %s", rawURL)

cannot have query parameters in url

[pawanrawal]

Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @arijitAD, @MichaelJCompton, and @pawanrawal)