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

Docs: Update Gharial error codes (DocuBlocks) #10760

Merged
merged 4 commits into from
Feb 25, 2021

Conversation

Simran-B
Copy link
Contributor

From the 3.6 release notes:

The General Graph document API is now consistent with the document API in its error messages.
When attempting to create / modify edges pointing to non existing vertex collections
HTTP 400 is returned instead of 404.

It only says vertex collection here. Is it still a 404 error if the vertex document does not exist, or is it 400 in both cases, vertex collection or vertex document missing?

From PATCH /_api/document description:

@RESTRETURNCODE{404}
is returned if the collection or the document was not found.

For the multi-document cases, it only says

@RESTRETURNCODE{404}
is returned if the collection was not found.

... which is probably because an array is returned in all cases (may contain error codes, I assume 404 if a document wasn't found?)

Experiments

404 or 400 if vertex/edge collection is missing? (tested with 3.6.0-rc.1 gf7a295252a)

  • add edge definition? POST /_api/gharial/{graph}/edge --> creates collections
  • replace edge definition? PUT /_api/gharial/{graph}/edge/{definition} --> creates vertex collections, throws 404 if edge coll not found
  • create an edge? POST /_api/gharial/{graph}/edge/{collection} --> 404 if vertex missing, 404 if vertex coll missing (with error collection 'edge coll' not found?!)
  • remove edge? DELETE /_api/gharial/{graph}/edge/{collection}/{edge} --> 404 if edge not found (with error edge collection not used in graph?!), same erorr if edge coll is not found
  • modify edge? PATCH /_api/gharial/{graph}/edge/{collection}/{edge} --> changes _from/_to without checking if the vertex collection or vertex exists!!!
  • replace edge? PUT /_api/gharial/{graph}/edge/{collection}/{edge} --> 404 if vertex not found, but 400 no collection name specified (?!) if vertex collection is missing
  • get edge? GET /_api/gharial/{graph}/edge/{collection}/{edge} --> 404 if edge not found
  • list vertex collections? GET /_api/gharial/{graph}/vertex --> 404 if graph not found
  • add vertex collection? POST /_api/gharial/{graph}/vertex --> 500 VPack error with {} body (probably other routes as well), 400 if already in use by graph (but with error collection used in orphans)
  • remove vertex collection? DELETE /_api/gharial/{graph}/vertex/{collection} --> 400 if coll is not part of graph (with error not in list of orphan collections),
  • create vertex? POST /_api/gharial/{graph}/vertex/{collection} --> 404 collection or view not found
  • remove a vertex? DELETE /_api/gharial/{graph}/vertex/{collection}/{vertex} --> 404 collection or view not found if vertex collection is missing, 404 document not found if vertex is missing
  • get a vertex? GET /_api/gharial/{graph}/vertex/{collection}/{vertex} --> 404 collection or view not found if vertex collection is missing, 404 document not found if vertex is missing
  • update vertex? PATCH /_api/gharial/{graph}/vertex/{collection}/{vertex} --> 404 collection or view not found if vertex collection is missing, 404 document not found if vertex is missing
  • replace vertex? PUT /_api/gharial/{graph}/vertex/{collection}/{vertex} --> 404 collection or view not found if vertex collection is missing, 404 document not found if vertex is missing

some of the error messages are misleading, but PATCH /_api/gharial/{graph}/edge/{collection}/{edge} not checking the _from / _to attribute whether the vertex document exists or not is a real bug.

@Simran-B
Copy link
Contributor Author

Could not reproduce PATCH /_api/gharial/{graph}/edge/{collection}/{edge} bug with more recent 3.6 server build (67e3578).

Some of the error messages are not very helpful or even incorrect nonetheless, needs further investigation.

@Simran-B Simran-B removed the 3.6.0 label Dec 21, 2019
@Simran-B
Copy link
Contributor Author

Simran-B commented Jun 3, 2020

Related: #11730

WIP script for testing the return codes:

#!/bin/bash

# Config
server="http://0.0.0.0:8520"
cred="root:"

# Internal
prevDescription=""
echo

test_route() {
    local description="$1"
    local method="$2"
    local path="$3"
    local body="$4"
    local comment="$5"

    if [ "$description" != "$prevDescription" ]; then
        echo "$description"
    fi
    echo -e "  \e[36m$method $path \e[35m$body\e[0m"
    # jq 'with_entries(select([.key] | inside(["code","errorMessage"])))'
    res=$(curl -s -u "$cred" --data-binary "$body" -X "$method" "$server$path" | jq --raw-output '(.code | tostring) + ": " + .errorMessage' 2>&1)
    echo -e "  \e[31m$res\e[0m  $comment\n"
    prevDescription="$description"
}

# Tests

test_route "Graph Create"              "POST"    "/_api/gharial"                         '{"name": "g"}'  "ok"
test_route "Graph Create Sync"         "POST"    "/_api/gharial?waitForSync=true"        '{"name": "gs"}' "ok"
test_route "Edge Definition Add"       "POST"    "/_api/gharial/g/edge"                  '{"collection": "ecoll", "from": ["vcoll"], "to": ["vcoll"]}'  "ok"
test_route "Edge Definition Add"       "POST"    "/_api/gharial/g/edge"                  '{"collection": "ecoll", "from": 1, "to": 2}'  "500 expected array for from / to"
test_route "Edge Definition Add"       "POST"    "/_api/gharial/g/edge"                  '{"collection": "ecoll", "from": [""], "to": [""]}'  "500 multi-use of edge coll"
test_route "Edge Definition Add"       "POST"    "/_api/gharial/g/edge"                  '{"collection": "ecoll2", "from": [""], "to": [""]}'  "400 illegal vertex collection name"
test_route "Edge Definition Replace"   "PUT"     "/_api/gharial/g/edge/ecoll"            '{"collection": "ecoll", "from": ["vcoll"], "to": ["vcoll"]}'  "ok"
test_route "Edge Definition Replace"   "PUT"     "/_api/gharial/g/edge/missing-ecoll"    '{"collection": "ecoll", "from": ["vcoll"], "to": ["vcoll"]}'  "ok"
test_route "Edge Definition Remove"    "DELETE"  "/_api/gharial/g/edge/missing-ecoll"    ''  "seems ok"
test_route "Edge Definition Remove"    "DELETE"  "/_api/gharial/missing/edge/ecoll"      ''  "ok, 404 graph not found"
# Edge Definition Remove???
#test_route "Vertex Collection Add"     "POST"    "/_api/gharial/g/vertex"                '{}'  "500 VPack error with {} body (probably other routes as well), 400 if already in use by graph (but with error collection used in orphans)"
#test_route "Vertex Collections List"   "GET"     "/_api/gharial/g/vertex"                '{}'  "404 if graph not found"
#test_route "Vertex Collection Remove"  "DELETE"  "/_api/gharial/g/vertex/vcoll"          '{}'  "400 if coll is not part of graph (with error not in list of orphan collections),"
test_route "Vertex Create"             "POST"    "/_api/gharial/g/vertex/vcoll"          '{"_key": "v1"}'  "ok"
#test_route "Vertex Create"             "POST"    "/_api/gharial/g/vertex/vcoll"          '{}'  "404 collection or view not found"
#test_route "Vertex Get"                "GET"     "/_api/gharial/g/vertex/vcoll/v1"       '{}'  "404 collection or view not found if vertex collection is missing, 404 document not found if vertex is missing"
#test_route "Vertex Update"             "PATCH"   "/_api/gharial/g/vertex/vcoll/v1"       '{}'  "404 collection or view not found if vertex collection is missing, 404 document not found if vertex is missing"
#test_route "Vertex Replace"            "PUT"     "/_api/gharial/g/vertex/vcoll/v1"       '{}'  "404 collection or view not found if vertex collection is missing, 404 document not found if vertex is missing"
#test_route "Vertex Remove"             "DELETE"  "/_api/gharial/g/vertex/vcoll/v1"       '{}'  "404 collection or view not found if vertex collection is missing, 404 document not found if vertex is missing"
test_route "Edge Create"               "POST"    "/_api/gharial/g/edge/ecoll"            '{"_key": "e1"}'  "ok, missing _from / _to"
test_route "Edge Create"               "POST"    "/_api/gharial/g/edge/ecoll"            '{"_key": "e1", "_from": "invalid", "_to": "vcoll/v1"}'  "ok, malformed _from / _to"
test_route "Edge Create"               "POST"    "/_api/gharial/g/edge/ecoll"            '{"_key": "e1", "_from": "vcoll/missing", "_to": "vcoll/v1"}'  "ok, missing vertex"
test_route "Edge Create"               "POST"    "/_api/gharial/g/edge/ecoll"            '{"_key": "e1", "_from": "missing/v1", "_to": "vcoll/v1"}'  "vertex coll missing, but error reports missing edge collection?!"
test_route "Edge Create"               "POST"    "/_api/gharial/g/edge/ecoll"            '{"_key": "e1", "_from": "vcoll/v1", "_to": "vcoll/v1"}'  "ok"
#test_route "Edge Get"                  "GET"     "/_api/gharial/g/edge/ecoll/e1"         '{}'  "404 if edge not found"
#test_route "Edge Update"               "PATCH"   "/_api/gharial/g/edge/ecoll/e1"         '{}'  "changes _from/_to without checking if the vertex collection or vertex exists!!!"
#test_route "Edge Replace"              "PUT"     "/_api/gharial/g/edge/ecoll/e1"         '{}'  "404 if vertex not found, but 400 no collection name specified (?!) if vertex collection is missing"
#test_route "Edge Remove"               "DELETE"  "/_api/gharial/g/edge/ecoll/e1"         '{}'  "404 if edge not found (with error edge collection not used in graph?!), same erorr if edge coll is not found"
test_route "Graph Remove"              "DELETE"  "/_api/gharial/g?dropCollections=true"  ''  "ok"
test_route "Graph Remove Sync"         "DELETE"  "/_api/gharial/gs?dropCollections=true"  '{"waitForSync": true}'  "not as documented?!"

@Simran-B
Copy link
Contributor Author

Simran-B commented Feb 5, 2021

POST /_api/gharial/my_graph/edge/my_edge_coll with payload
{"_key": "e1", "_from": "MISSING/v1", "_to": "my_vert_coll/v1"} reports 404: collection 'my_edge_coll' not found even though the vertex collection is missing.

The request succeeds if I use a valid vertex collection, which proves that the edge collection is fine.
{"_key": "e1", "_from": "my_vert_coll/v1", "_to": "my_vert_coll/v1"}

If an edge definition is not found (path parameter), then 404: edge collection not used in graph is returned.

Only half of the PR changes are true right now, waiting for bug fix / confirmation on missing vertex collection issue.

There are still some include DocuBlocks with old links, but they appear to be unused.
@Simran-B Simran-B removed the 9 WIP label Feb 25, 2021
@Simran-B Simran-B marked this pull request as ready for review February 25, 2021 09:56
@Simran-B
Copy link
Contributor Author

https://arangodb.atlassian.net/browse/BTS-309 introduced a new error to clearly state when a referenced vertex collection does not exist and it's now a 400 error as was stated in the 3.6 release notes (but apparently never implemented?)

@Simran-B Simran-B merged commit c2a6794 into devel Feb 25, 2021
@Simran-B Simran-B deleted the documentation/gharial-error-codes branch February 25, 2021 10:04
Simran-B added a commit that referenced this pull request Feb 25, 2021
With BTS-309 fixed it's 400 if vertex coll not found, but still 400 if vertex not found.

Remove legacy links from Rest DocuBlocks. There are still some include DocuBlocks with old links, but they appear to be unused.
Simran-B added a commit that referenced this pull request Feb 25, 2021
With BTS-309 fixed it's 400 if vertex coll not found, but still 400 if vertex not found.

Remove legacy links from Rest DocuBlocks. There are still some include DocuBlocks with old links, but they appear to be unused.
KVS85 pushed a commit that referenced this pull request Mar 7, 2021
With BTS-309 fixed it's 400 if vertex coll not found, but still 400 if vertex not found.

Remove legacy links from Rest DocuBlocks. There are still some include DocuBlocks with old links, but they appear to be unused.
KVS85 pushed a commit that referenced this pull request Mar 7, 2021
With BTS-309 fixed it's 400 if vertex coll not found, but still 400 if vertex not found.

Remove legacy links from Rest DocuBlocks. There are still some include DocuBlocks with old links, but they appear to be unused.
elfringham pushed a commit to elfringham/arangodb that referenced this pull request Apr 20, 2021
With BTS-309 fixed it's 400 if vertex coll not found, but still 400 if vertex not found.

Remove legacy links from Rest DocuBlocks. There are still some include DocuBlocks with old links, but they appear to be unused.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant