`/status` endpoint does not follow the "Standard Response Format" found in API spec
hudson-newey opened this issue · 3 comments
Currently, the api returns the /status
endpoint returns an object that doesn't follow the response format outlined in the baw-api "Standard Response Format".
Current Response Format:
{
"status": "good" | "bad",
"timed_out": boolean,
"database": boolean,
"redis": "PONG" | "",
"storage": "No audio recording storage directories are available." | `${number} audio recording storage directory available.`,
"upload": "Alive" | "Dead"
}
This is a problem since it doesn't have the meta
and data
properties expected from the spec structure. This means that this specific endpoint requires special client-side service conditions to work (since our abstract-service expects the content to be under the data
property).
Expected response:
{
"meta": {
"status": 200,
"message": "OK"
},
"data": {
"status": "good" | "bad",
"timed_out": boolean,
"database": boolean,
"redis": "PONG" | "",
"storage": "1 audio recording storage directory available." | `${number} audio recording storage directory available.`,
"upload": "Alive" | "Dead"
}
}
Additionally, we might want to change status
, redis
, and storage
properties to boolean
types. This would make the object more readable.
If we want to retain information such as the number of storage directories available, we could emit a sub-model similar to the meta
property (where we have an object with status and message properties).
We might also consider changing the timed_out
key to something like server_connected
so it isn't a negated property. At the moment redis = true, database = true, indicates that the connection is healthy, but timed_out = true means the connection is unhealthy. By changing it to sever_connected
(or similar) it makes it easier to read since a true property is good, while a false property is bad.
I'm expecting this issue to turn into a discussion, so any feedback would be appreciated.
The API originally didn't follow the standard format because
a) back then we didn't have a standard
and mostly b) it was designed for the simplest possible parsing
As for the fields, most have the ability to include some kind of error message. They're not Boolean fields. In fact even the status field (which is Boolean) was made a string so it could be unambiguously matched with a simple grep.
I'm not opposed to changes, but they would be breaking.
For the fields we could make them unions boolean | string
or each with the possibility of producing an error message could be of the type IResult { error: string?, alive: boolean }
.
The API originally didn't follow the standard format because
That makes sense. Since it's a working feature and sounds like it will generate a lot of breaking changes, I'd like to put this on the backlog as a low-priority / stretch goal.
For the fields we could make them unions boolean | string or each with the possibility of producing an error message could be of the type IResult { error: string?, alive: boolean }.
Out of the two options, I'd prefer the overloading of types because:
a. It might might maintain backward compatibility with some scripts
b. I'm a bit hesitant about the IResult { error: string?, alive: boolean }
structure as the error?: string
is not in convention with our error spec
I'm a bit hesitant about the IResult { error: string?, alive: boolean } structure as the error?: string is not in convention with our error spec
Incorrect. The error spec for a start is singular (we have multiple possible error messages). Secondly, the error information is always for a request that has failed. A status request can successfully report the statuses and error messages of many components, without the status request itself failing.