error message update

[UlfBj]

Below are my rationale for the changes. 304 not_modified

What would lead to this response? A Set that fails should have a different error code.

400 filter_invalid

This is covered by 400 bad_request or invalid_data

400 invalid_duration

This is covered by 400 invalid_data

400 invalid_value

This is replaced by 400 invalid_data

401 too_many_attempts

Authentication is validated by the AGTS, so this is not visible in this context.

401 read_only

This is covered by 401 invalid_token.

403 user_unknown

User identity is only explicit (in client context) in a token. This error would therefore be caught in the authentication step.

403 user_forbidden

User identity is only explicit (in client context) in a token. This error would therefore be caught in the authentication step.

403 device_forbidden

Device identity is only explicit (in client context) in a token. This error would therefore be caught in the authentication step.

403 device_unknown

Device identity is only explicit (in client context) in a token. This error would therefore be caught in the authentication step.

404 invalid_path

This is covered by 400 invalid_data

404 private_path

This is not a relevant scenario.

404 invalid_subscription_id

This is covered by 400 invalid_data

406 insufficient_priviledges

This is covered by 401 invalid_token

406 not_acceptable

Is this the role of the server to judge?

429 too_many_requests

This is covered by 503 service_unavailable.

502 bad_gateway

Is this a relevant scenario?

504 gateway_timeout

Is this a relevant scenario?

[UlfBj]

A rendered version of the Transport spec: https://rawcdn.githack.com/UlfBj/automotive/b373ac4387360c3f5fc54eae47a61ae444899817/spec/VISSv2_Transport.html

[erikbosch]

Do we intend to cover rejection due to safety reasons in VISS. I have seen other proposals of using 409 (Conflict) if something cannot be done due to safety reasons, like open door is prevented due to vehicle moving.

[UlfBj]

Regarding the error code 409, it is specified in RFC2616, for which the spec says The client SHOULD support any status code defined in RFC2616 So I think we have it sufficiently covered. But if we think it should be highlighted I am fine with adding it to the list.

[tguild]

I am wondering if we are perhaps overloading error codes with multiple meanings too much and if there isn't a good match in HTTP and instead create additional codes. Also feel like we might have circled around this before and even requested/received advice that it was reasonable. Perhaps someone has a better recollection of that conversation @caribouW3 may recall. I can also look for old issues and minutes on that.

What I am certain would not be a misuse of response codes is:

NNN immutable_descriptor "additional free form text providing additional details"

examples:

404 Document not found "The VSS signal name requested does not exist on this server"

or since Ulf prefers considering that an invalid request

400 Bad request "The VSS signal name requested does not exist on this server"

Btw on 401 vs 403,

401 is the initial authentication needed response and can also be returned if given improper credentials, prompting the client to submit different ones. This can be a looping scenario the server needs to be careful about (stop responding to client if receiving too many requests/minute which also protects against brute force attacks). Client can break the loop too, think of the now seldom seen basic/digest authentication browser pop up for username/password (html forms, certificates etc currently more common instead on the web) and instead of providing credentials and submitting with ok, hitting cancel. At that point you get a 403.

403 is for when permissions aren't set or client provided credentials that are not authorized to access the resource.

and multiple 403s would be

403 Forbidden "Device not permitted" (btw not sure it is important to distinguish an unknown from a non-permitted device) 403 Forbidden "User not permitted"

https://developer.mozilla.org/en-US/docs/Web/HTTP/Status

[erikbosch]

I took a second look on this PR and the result after it was merged on realized that there are some more changes that we better shall do to make things consistent. First, we should review the examples in VISSv2 Transport, where we today sometimes use error numbers and error reasons not listed in the table. Allowed, but not maybe optimal, we should better have examples with the "recommended" error reasons.

Concerning the comment from Ted, isn't the "official" reason for each status code is "hard-coded", i.e. that we see "Not Found" below is "mandatory" as the error code is 404. But for reason and message in the error body we are free to select whatever we want, but depending on implementation I assume that we not always will get an error body, for example if the error is given by the server framework rather than the VISS application.

HTTP/1.1 404 Not Found
              Content-Type: application/json; charset=utf-8
          ...
              {
                "error": {"number": 404, "reason": "invalid_path", "message": "The specified data path does not exist."},
                "ts": "2020-04-15T13:37:00Z"
              }

Then we actually mention explicit error codes here and there in the core spec. Maybe we should take a second look at them as well.

https://raw.githack.com/w3c/automotive/gh-pages/spec/VISSv2_Core.html#access-control-server

[UlfBj]

Updating the examples is a good idea.

Regarding having an error code that explicitly addresses paths I think leads to an "unbalance" if that level of detail is not used also for other data elements e.g. in filter expressions etc. In the current error list all data elements are addressed by either invalid_data or unavailable_data.