Unsupported basearch leads to 500 internal error

[tomleb]

Not sure if that's the expected behavior, but sending a request to cincinnati with a non-supported basearch leads to a 500 internal error.

For example:

$ curl  --header 'Accept:application/json' 'https://updates.coreos.fedoraproject.org/v1/graph?stream=stable&basearch=mock-amd64'
HTTP status server error (500 Internal Server Error) for url (http://127.0.0.1:8080/v1/graph?basearch=mock-amd64&stream=stable)

The issue seems to be here: https://github.com/coreos/fedora-coreos-cincinnati/blob/master/fcos-graph-builder/src/main.rs#L147

    let arch_graph = policy::pick_basearch(cached_graph, basearch)?;
    let final_graph = policy::filter_deadends(arch_graph);

Instead of returning an HttpResponse, we're just erroring out.

I haven't used rust in ages, but I can attempt a fix. Should we send a 400 bad request?

[lucab]

Sorry for the delay, I didn't see the notification for this. I'm not opposed to it but I wonder what the actual problem with a 500 is? According to https://github.com/coreos/zincati/blob/master/docs/development/cincinnati/protocol.md#errors all of 4xx and 5xx are fine as error status.

[tomleb]

To be honest, I hadn't seen that protocol file before creating this issue. The protocol does not seem to explicitly call for "proper" 400 errors and 500 errors. Since I'm just used to having 400 errors for request errors and 500 errors for server error, I was expecting a 400 error when I ran the query. I still think it's better to send http errors that make sense (eg: bad request when it's a bad request). If that's not important for this project though that's also fine.

[lucab]

@tomleb no problems, happy to merge that if it makes the flow less surprising, I was just missing the whole context about the ticket thus asking for more hints :)