New error type values

[ayuhito]

Following up on #202, I think having unique type values for every error is great for users that depend on the API programmatically.

Currently, 400 - no probes found returns "type": "api_error" which is the same as the Internal Server Error type. Does "type": "no_probes_found" sound suitable?

I also think we should change the 422 - validation error which returns "type": "invalid_request_error" to "type": "validation_error" to be more specific.

[patrykcieszkowski]

• api_error should be reserved for 5XX errors
• both invalid_request_error and validation_error mean pretty much the same thing. They're caused by user error. I don't mind either.
• I've mixed feelings when it comes to no probes error though. Is it really a user error? It should return 404 for sure.

[MartinKolarik]

api_error should be reserved for 5XX errors

I'll reiterate from #202 that unless each specific error has a different type, that property is useless because you can just use the status code. The only reason for having type is having different values for different error causes that fall under the same HTTP code.

[patrykcieszkowski]

And since we can't expose server errors, it should stay generic.

[MartinKolarik]

I've mixed feelings when it comes to no probes error though. Is it really a user error? It should return 404 for sure.

The closest is 422, and then a bit more generic but still correct is 400. Asking for a non-existing location is roughly the same as asking for a non-existing test type.

A 404 in response to the POST would mean the whole /measurements endpoint is gone - regardless of the payload.

[patrykcieszkowski]

Asking for a non-existing location is roughly the same as asking for a non-existing test type.

It's not, since we don't guarantee GEO availability - its not dependent on us.

[MartinKolarik]

Yes, there's a small difference, but it's still the closest from the available options of status codes (also we do provide a list of all possible correct values at /probes).

[MartinKolarik]

One alternative that somewhat makes sense: create the measurement as usual, with 0 probes and status: finished. It would match how we handle all other situations when some probes are found but not all.

E.g., right now, this is not an error and one of the locations get silently ignored (or should this be an error?):

{
  "type":"ping",
  "target":"google.com",
  "measurementOptions":{

  },
  "locations":[
    {
      "city":"xxxx",
      "limit":1
    },
    {
      "city":"Prague",
      "limit":1
    }
  ]
}

[patrykcieszkowski]

also we do provide a list of all possible correct values at /probes

Yes, but they can disconnect at any time; just like they're unavailable soon after the API restart. Error 400 implies the body of the request was incorrect, while in reality it wasn't. But I admit, that probably wouldn't be a big issue since we describe the error.

The code 404 doesn't mean the URL was not found, but rather it indicates the resource was unavailable. In our case - Locations are the resources.

RFC 7231:

[6.5.4](https://www.rfc-editor.org/rfc/rfc7231#section-6.5.4).  404 Not Found

   The 404 (Not Found) status code indicates that the origin server did
   not find a current representation for the target resource or is not
   willing to disclose that one exists.  A 404 status code does not
   indicate whether this lack of representation is temporary or
   permanent; the 410 (Gone) status code is preferred over 404 if the
   origin server knows, presumably through some configurable means, that
   the condition is likely to be permanent.

   A 404 response is cacheable by default; i.e., unless otherwise
   indicated by the method definition or explicit cache controls (see
   [Section 4.2.2 of [RFC7234]](https://www.rfc-editor.org/rfc/rfc7234#section-4.2.2)).

One alternative that somewhat makes sense: create the measurement as usual, with 0 probes and status: finished. It would match how we handle all other situations when some probes are found but not all.

Yes, I was considering that too. I'm torn on whether that's a good idea.

[MartinKolarik]

The 404 (Not Found) status code indicates that the origin server did not find a current representation for the target resource or is not willing to disclose that one exists.

That resource is /v1/measurements, and returning 404 would indicate the whole collection of measurements doesn't exist. It isn't related to the payload.

[MartinKolarik]

Yes, I was considering that too. I'm torn on whether that's a good idea.

I'm not entirely sure yet as well, but we can IMO look at this from two perspectives.

1. The client is responsible for choosing the correct locations. It has the means to do so (ignoring probe disconnects as an edge case). In this case, wrong locations receive 422, the description of which matches exactly the condition "Unprocessable Entity"

   the server understands the content type of the request entity, and the syntax of the request entity is correct, but it was unable to process the contained instructions

2. The client indicates what locations it wants, and the server does the best it can to match it - but it's just an indication and no 100% match is guaranteed. In this case, a 200 with empty result would be an ok response.

[patrykcieszkowski]

The 404 (Not Found) status code indicates that the origin server did not find a current representation for the target resource or is not willing to disclose that one exists.

That resource is /v1/measurements, and returning 404 would indicate the whole collection of measurements doesn't exist. It isn't related to the payload.

Only if you strictly follow the classic CURD methodology. Which GlobalPing doesn't - if it did, the location would be contained as part of the URL. That's why POST requests aren't cached by default too, btw.

[patrykcieszkowski]

2. The client indicates what locations it wants, and the server does the best it can to match it - but it's just an indication and no 100% match is guaranteed. In this case, a 200 with empty result would be an ok response.

We can add an additional key to the POST response with failed requests.

[MartinKolarik]

Only if you strictly follow the classic CURD methodology. Which GlobalPing doesn't - if it did, the location would be contained as part of the URL. That's why POST requests aren't cached by default too, btw.

No, it's how that term is used in the whole RFC. There's even a section on this in 7230:

5.1. Identifying a Target Resource

...

   HTTP communication is initiated by a user agent for some purpose.
   The purpose is a combination of request semantics, which are defined
   in [[RFC7231](https://www.rfc-editor.org/rfc/rfc7231)], and a target resource upon which to apply those
   semantics. 

   A URI reference ([Section 2.7](https://www.rfc-editor.org/rfc/rfc7230#section-2.7)) is typically used as an
   identifier for the "target resource", which a user agent would
   resolve to its absolute form in order to obtain the "target URI".
   The target URI excludes the reference's fragment component, if any,
   since fragment identifiers are reserved for client-side processing
   ([[RFC3986], Section 3.5](https://www.rfc-editor.org/rfc/rfc3986#section-3.5)).

404 is imply not suitable here.

[MartinKolarik]

Also, note that 404 is a "user error" just as much as any other 4xx code. If we went with "wrong location is not a user error" logic, we would have to discuss either 5xx options (server errors) or 2xx (no errors at all).

[patrykcieszkowski]

And HTTP was designed with CRUD actions in mind. The section you've quoted clearly states, the URI is typically used as the target resource, implying that's not always the case. Again, if we were to strictly follow RFC - we would make users to send a POST request to the location builder resource, and use the returned hash/id (resource) as part of the URI for the actual measurement request. Or we would make them include locations as part of the querystring (still URI). We don't, and therefore we're unable to use the same range of HTTP codes as were intended.

Yes, 4XX codes are classified as user errors. But there's a huge difference between requesting a resource that's not there, or and incorrectly formatting the request body.

Let's just use 418 Im a teapot and we'll all be happy.

[MartinKolarik]

But there's a huge difference between requesting a resource that's not there, or and incorrectly formatting the request body.

400/422 are both not only about formatting but also about semantics, which is why I consider them acceptable. A 200 might be the cleanest, and I'd be ok with it, but 422 allows us to better explain what happens, which I think makes it better in practice.