abedra / libvault

A C++ library for Hashicorp Vault
MIT License
34 stars 25 forks source link

Somehow notify client of Vault error #79

Closed zspasztori closed 3 years ago

zspasztori commented 3 years ago

When there is an error related to curl, it can be returned with HttpErrorCallback.

However if the error happens in HttpConsumer level, this is the case when the Vault server returns an error code, the error is lost. The only thing we know is that the error happened, since the std::optional field is not set.

I think it would be very useful to be able to query this error in someform.

abedra commented 3 years ago

An additional callback could be created and passed into construction just like the error callback. In fact, it could even be the same callback if desired. The plumbing mostly exists to make this possible, but there's a bit of tedium around pushing that callback through the various http consumer methods and calling it. That being said, it's mostly just plumbing and shouldn't be terribly difficult to pull off. I would probably create a separate alias for this callback to make it more obvious that we are separating error and failure semantics though.

zspasztori commented 3 years ago

I would use a single callback function since it takes string anyway. I would create a function which would invoke the callback and return std::nullopt. Pass it the callback straight from client. This would result in a minimal change of the current design.

It would look something like this:

return HttpClient::is_success(response) ? std::optional<std::string>(response.value().body.value()) : invoke_callback(client.httpErrorCallback);

What is your opinion on this?