Add line and colum numbers to JSON errors thrown by HTTP endpoints. #4012

martinmr · 2019-09-17T22:32:05Z

Implementation idea taken from
https://adrianhesketh.com/2017/03/18/getting-line-and-character-positions-from-gos-json-unmarshal-errors/

Fixes #3965

This change is

Implementation idea taken from https://adrianhesketh.com/2017/03/18/getting-line-and-character-positions-from-gos-json-unmarshal-errors/

pullrequest

✅ A review job has been created and sent to the PullRequest network.

@martinmr you can click here to see the review status or cancel the code review job.

martinmr · 2019-09-17T22:32:21Z

Example output:

➜  dgraph-linux-amd64 curl -H "Content-Type: application/json" -X POST "localhost:8180/mutate?commitNow=true" -d '{
  "set": [
    {
      "uid": "0x1",
      "friends_with",: [
        {
          "uid": "0x3",
          "friends_with|good": false,
        }
      ]
    }
  ]
}'
{"errors":[{"message":"Error parsing JSON at line 5, character 23: invalid character ',' after object key\n","extensions":{"code":"ErrorInvalidRequest"}}]}%

pullrequest

Overall looks good and conforms to description. Added comments inline for simplification and adding the link for future reference.

Reviewed with ❤️ by PullRequest

pullrequest · 2019-09-18T00:14:19Z

dgraph/cmd/alpha/http.go

@@ -312,7 +313,8 @@ func mutationHandler(w http.ResponseWriter, r *http.Request) {
 	case "application/json":
 		ms := make(map[string]*skipJSONUnmarshal)
 		if err := json.Unmarshal(body, &ms); err != nil {
-			x.SetStatus(w, x.ErrorInvalidRequest, err.Error())
+			jsonErr := convertJSONError(string(body), err)


It seems these lines are duplicated across multiple functions -- I suggest creating another wrapper which encapsulates the two lines you added here and other places

pullrequest · 2019-09-18T00:14:19Z

dgraph/cmd/alpha/http.go

@@ -577,3 +579,52 @@ func (sju *skipJSONUnmarshal) UnmarshalJSON(bs []byte) error {
 	sju.bs = bs
 	return nil
 }
+
+// convertJSONError adds line and character information to the JSON error.


I would add a link to the article as a part of the comment for reference

martinmr

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @campoy, @manishrjain, and @pullrequest[bot])

dgraph/cmd/alpha/http.go, line 316 at r1 (raw file):

Previously, pullrequest[bot] wrote…

It seems these lines are duplicated across multiple functions -- I suggest creating another wrapper which encapsulates the two lines you added here and other places

Not doing this for just a couple of lines.

dgraph/cmd/alpha/http.go, line 583 at r1 (raw file):

Previously, pullrequest[bot] wrote…

I would add a link to the article as a part of the comment for reference

Done.

manishrjain

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @campoy and @pullrequest[bot])

Add line and colum numbers to JSON errors thrown by HTTP endpoints.

eaa4c6b

Implementation idea taken from https://adrianhesketh.com/2017/03/18/getting-line-and-character-positions-from-gos-json-unmarshal-errors/

martinmr requested review from manishrjain and a team as code owners September 17, 2019 22:32

pullrequest bot reviewed Sep 17, 2019

View reviewed changes

martinmr requested a review from campoy September 17, 2019 22:32

pullrequest bot reviewed Sep 18, 2019

View reviewed changes

martinmr commented Sep 18, 2019

View reviewed changes

Add link to blog post.

f800462

manishrjain approved these changes Sep 27, 2019

View reviewed changes

martinmr merged commit 3039115 into master Sep 27, 2019

martinmr deleted the martinmr/json-error-lines branch September 27, 2019 18:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add line and colum numbers to JSON errors thrown by HTTP endpoints. #4012

Add line and colum numbers to JSON errors thrown by HTTP endpoints. #4012

martinmr commented Sep 17, 2019 •

edited by manishrjain

Loading

pullrequest bot left a comment

martinmr commented Sep 17, 2019

pullrequest bot left a comment

pullrequest bot Sep 18, 2019

pullrequest bot Sep 18, 2019

martinmr left a comment

manishrjain left a comment

Add line and colum numbers to JSON errors thrown by HTTP endpoints. #4012

Add line and colum numbers to JSON errors thrown by HTTP endpoints. #4012

Conversation

martinmr commented Sep 17, 2019 • edited by manishrjain Loading

pullrequest bot left a comment

Choose a reason for hiding this comment

martinmr commented Sep 17, 2019

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot Sep 18, 2019

Choose a reason for hiding this comment

pullrequest bot Sep 18, 2019

Choose a reason for hiding this comment

martinmr left a comment

Choose a reason for hiding this comment

manishrjain left a comment

Choose a reason for hiding this comment

martinmr commented Sep 17, 2019 •

edited by manishrjain

Loading