REST++ API: Failed to convert user vertex id for parameter, but the response code is 200 (OK)

In cases of queries that accept a vertex input parameter, when the input doesn’t translate to actual vertex, we’ll get an error "“Failed to convert user vertex id for parameter personId”.

For example:

 curl -v  "http://localhost:9000/query/LDBC_SNB/interactiveComplex1?personId=4398046511333&firstName=Jose"
*   Trying 127.0.0.1:9000...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 9000 (#0)
> GET /query/LDBC_SNB/interactiveComplex1?personId=4398046511333&firstName=Jose HTTP/1.1
> Host: localhost:9000
> User-Agent: curl/7.68.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Server: nginx
< Date: Mon, 10 Jan 2022 15:11:35 GMT
< Content-Type: application/json; charset=utf-8
< Content-Length: 139
< Connection: keep-alive
< GSQL-Task-Key: 16842829.RESTPP_1_1.1641827495045.N
< GSQL-Task-Time: 1 ms
< Strict-Transport-Security: max-age=63072000; includeSubdomains; preload
<
* Connection #0 to host localhost left intact
{"version":{"edition":"enterprise","api":"v2","schema":0},"error":true,"message":"Failed to convert user vertex id for parameter personId"}

Please note the HTTP response code, though. It is 200, as if the request was OK.

I think it would be more appropriate to return an error code in this case (for example 404, or 400).

Hi gruby_karol,

The error code is contained in the body of the return: "error":true.

The request returns a 200 because it is still a successful REST request, the TG server processed the information and returned the request, just the execution of the query was unsuccessful and the REST return successfully reports that status.

Dan, it is just my suggestion to make things more predictable, so I won’t push here.

But let’s try to analyze the following request, using the reasoning you provided:

 curl -v  http://localhost:9000/LDBC_SNB/non-existing
*   Trying 127.0.0.1:9000...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 9000 (#0)
> GET /LDBC_SNB/non-existing HTTP/1.1
> Host: localhost:9000
> User-Agent: curl/7.68.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 404 Not Found
< Server: nginx
< Date: Mon, 10 Jan 2022 20:19:38 GMT
< Content-Type: application/json; charset=utf-8
< Content-Length: 214
< Connection: keep-alive
< GSQL-Task-Key: EndpointNotExist
< GSQL-Task-Time: 1641845978463 ms
<
* Connection #0 to host localhost left intact
{"version":{"edition":"enterprise","api":"v2","schema":0},"error":true,"message":"Endpoint is not found from url = /LDBC_SNB/non-existing, please use GET /endpoints to list all valid endpoints.","code":"REST-1000"}

We have a successful REST request, we see "error":true and the error message.
So, why return “< HTTP/1.1 404 Not Found” instead of 200?

Again, in my opinion, it would be more consistent to give error responses proper error codes, but it is the developers’ choice how to design their API. And, I may be completely wrong in my opinion :wink:

Best regards,
Gruby

Hey Gruby,

Thanks for the additional example. I’m going to forward this info back to our development team as I now see where this could certainly be causing confusion and agree that there should be a consistent behavior among “non-existent” endpoints.

I’ll play “devil’s advocate” here to try to justify the current setup, but I do agree that this is something that we should further evaluate from a clarity standpoint or at the very least have additional documentation on.

Devil’s advocacy:
In this example with the non-existent endpoint, the request returns a 404 because the endpoint itself is not found so the request cannot be completed. In the prior example with the non-existent vertexID, the request does indeed go through, and the query does try to run, however the query fails, not the REST request. From that standpoint, the request was successful, hence the 200, but the query was not, so the "error":true.

I hope that clarifies the current process, and I’ll talk to our engineering team to re-evaluate that process for additional clarity in a future update.

Thanks,
Dan