Skip to content

Commit eea3d82

Browse files
Totto16Totto16
committed
fix: make SecretStorage return the errors instead of printing them directly
1 parent 85b7635 commit eea3d82

File tree

3 files changed

+50
-48
lines changed

3 files changed

+50
-48
lines changed

‎src/lobby/api.cpp‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,14 @@ lobby::API::API(const std::string& api_url)
5656
: m_client{ std::make_unique<oopetris::http::implementation::ActualClient>(api_url) },
5757
m_secret_storage{ std::make_unique<secret::SecretStorage>(secret::KeyringType::User) } {
5858

59-
if (auto value = m_secret_storage->load(constants::api_token_key); value.has_value()) {
59+
auto value = m_secret_storage->load(constants::api_token_key);
60+
61+
if (value.has_value()) {
6062
if (not this->setup_authentication(value.value())) {
6163
throw std::runtime_error("Couldn't setup authentication");
6264
}
65+
} else {
66+
spdlog::error("API {}", value.error());
6367
}
6468
}
6569

@@ -139,7 +143,9 @@ bool lobby::API::authenticate(const Credentials& credentials) {
139143
void lobby::API::logout() {
140144
m_client->ResetBearerAuth();
141145

142-
m_secret_storage->remove(constants::api_token_key);
146+
if (auto result = m_secret_storage->remove(constants::api_token_key); result.has_value()) {
147+
spdlog::error("API: {}", result.value());
148+
}
143149
}
144150

145151
helper::expected<std::vector<lobby::LobbyInfo>, std::string> lobby::API::get_lobbies() {
@@ -264,7 +270,9 @@ bool lobby::API::setup_authentication(const std::string& token) {
264270
m_authentication_token = token;
265271

266272
m_client->SetBearerAuth(token);
267-
m_secret_storage->store(constants::api_token_key, token);
273+
if (auto result = m_secret_storage->store(constants::api_token_key, token); result.has_value()) {
274+
spdlog::error("API {}", result.value());
275+
}
268276

269277
return true;
270278
}

‎src/lobby/credentials/secret.cpp‎

Lines changed: 34 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44
#if defined(__linux__)
55

66
#include <core/helper/utils.hpp>
7-
#include <spdlog/spdlog.h>
8-
9-
7+
#include <fmt/format.h>
108
namespace {
119

1210
namespace constants {
@@ -33,7 +31,7 @@ namespace {
3331

3432
secret::SecretStorage::SecretStorage(KeyringType type) : m_type{ type } {
3533

36-
key_serial_t key_type;
34+
key_serial_t key_type{};
3735
switch (m_type) {
3836
case secret::KeyringType::User:
3937
key_type = KEY_SPEC_USER_KEYRING;
@@ -43,8 +41,8 @@ secret::SecretStorage::SecretStorage(KeyringType type) : m_type{ type } {
4341
// -1 stands for current uid, 0 stands for: do not create a link to another keyring
4442
this->m_ring_id = keyctl_get_persistent(-1, 0);
4543
if (this->m_ring_id < 0) {
46-
spdlog::error("Error while getting the persistent keyring: {}", strerror(errno));
47-
return;
44+
throw std::runtime_error(fmt::format("Error while getting the persistent keyring: {}", strerror(errno))
45+
);
4846
}
4947

5048
return;
@@ -57,99 +55,93 @@ secret::SecretStorage::SecretStorage(KeyringType type) : m_type{ type } {
5755
this->m_ring_id = keyctl_get_keyring_ID(key_type, 1);
5856

5957
if (m_ring_id < 0) {
60-
spdlog::error("Error while getting the requested keyring: {}", strerror(errno));
61-
return;
58+
throw std::runtime_error(fmt::format("Error while getting the requested keyring: {}", strerror(errno)));
6259
}
6360
}
6461

6562

66-
[[nodiscard]] std::optional<std::string> secret::SecretStorage::load(const std::string& key) const {
63+
[[nodiscard]] helper::expected<std::string, std::string> secret::SecretStorage::load(const std::string& key) const {
6764

6865
if (m_ring_id < 0) {
69-
spdlog::error("Error while loading a key, ring_id is invalid");
70-
return std::nullopt;
66+
return helper::unexpected<std::string>{ "Error while loading a key, ring_id is invalid" };
7167
}
7268

73-
auto id = get_id_from_name(m_ring_id, key);
69+
auto key_id = get_id_from_name(m_ring_id, key);
7470

75-
if (id < 0) {
76-
spdlog::error("Error while loading a key, can't find key by name: {}", strerror(errno));
77-
return std::nullopt;
71+
if (key_id < 0) {
72+
return helper::unexpected<std::string>{
73+
fmt::format("Error while loading a key, can't find key by name: {}", strerror(errno))
74+
};
7875
}
7976

8077

8178
void* buffer = nullptr;
8279

83-
auto result = keyctl_read_alloc(id, &buffer);
80+
auto result = keyctl_read_alloc(key_id, &buffer);
8481

8582
if (result < 0) {
86-
spdlog::error("Error while loading a key, can't read the value: {}", strerror(errno));
87-
return std::nullopt;
83+
return helper::unexpected<std::string>{
84+
fmt::format("Error while loading a key, can't read the value: {}", strerror(errno))
85+
};
8886
}
8987

9088
auto result_string = std::string{ static_cast<char*>(buffer) };
9189
free(buffer);
9290
return result_string;
9391
}
9492

95-
void secret::SecretStorage::store(const std::string& key, const std::string& value, bool update) const {
93+
[[nodiscard]] std::optional<std::string>
94+
secret::SecretStorage::store(const std::string& key, const std::string& value, bool update) const {
9695

9796
if (m_ring_id < 0) {
98-
spdlog::error("Error while storing a key, ring_id is invalid");
99-
return;
97+
return "Error while storing a key, ring_id is invalid";
10098
}
10199

102-
auto id = get_id_from_name(m_ring_id, key);
100+
auto key_id = get_id_from_name(m_ring_id, key);
103101

104102

105-
if (id > 0 && not update) {
106-
spdlog::error("Error while storing a key, it already exists and can't update it");
107-
return;
103+
if (key_id > 0 && not update) {
104+
return "Error while storing a key, it already exists and can't update it";
108105
}
109106

110107
auto full_key = get_key_name(key);
111108

112109
auto new_id = add_key(constants::key_type_user, full_key.c_str(), value.c_str(), value.size(), m_ring_id);
113110

114111
if (new_id < 0) {
115-
spdlog::error("Error while storing a key, can't add the key: {}", strerror(errno));
116-
return;
112+
return fmt::format("Error while storing a key, can't add the key: {}", strerror(errno));
117113
}
118114

119115

120116
auto result = keyctl_link(new_id, m_ring_id);
121117

122118
if (result < 0) {
123-
spdlog::error("Error while storing a key, can't link the key to the keyring: {}", strerror(errno));
124-
return;
119+
return fmt::format("Error while storing a key, can't link the key to the keyring: {}", strerror(errno));
125120
}
121+
122+
return std::nullopt;
126123
}
127124

128125

129-
void secret::SecretStorage::remove(const std::string& key) const {
126+
[[nodiscard]] std::optional<std::string> secret::SecretStorage::remove(const std::string& key) const {
130127

131128
if (m_ring_id < 0) {
132-
spdlog::error("Error while remove a key, ring_id is invalid");
133-
return;
129+
return fmt::format("Error while remove a key, ring_id is invalid");
134130
}
135131

136-
auto id = get_id_from_name(m_ring_id, key);
132+
auto key_id = get_id_from_name(m_ring_id, key);
137133

138-
if (id < 0) {
139-
spdlog::error("Error while removing a key, can't find key by name: {}", strerror(errno));
140-
return;
134+
if (key_id < 0) {
135+
return fmt::format("Error while removing a key, can't find key by name: {}", strerror(errno));
141136
}
142137

143-
auto result = keyctl_unlink(id, m_ring_id);
138+
auto result = keyctl_unlink(key_id, m_ring_id);
144139

145140
if (result < 0) {
146-
spdlog::error("Error while removing a key, can't unlink key: {}", strerror(errno));
147-
return;
141+
return fmt::format("Error while removing a key, can't unlink key: {}", strerror(errno));
148142
}
149143

150-
151-
152-
144+
return std::nullopt;
153145
}
154146

155147

‎src/lobby/credentials/secret.hpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <optional>
55
#include <string>
66

7+
#include <core/helper/expected.hpp>
78
#include <core/helper/types.hpp>
89

910
#if defined(__linux__)
@@ -28,11 +29,12 @@ namespace secret {
2829
public:
2930
explicit SecretStorage(KeyringType type);
3031

31-
[[nodiscard]] std::optional<std::string> load(const std::string& key) const;
32+
[[nodiscard]] helper::expected<std::string, std::string> load(const std::string& key) const;
3233

33-
void store(const std::string& key, const std::string& value, bool update = true) const;
34+
[[nodiscard]] std::optional<std::string>
35+
store(const std::string& key, const std::string& value, bool update = true) const;
3436

35-
void remove(const std::string& key) const;
37+
[[nodiscard]] std::optional<std::string> remove(const std::string& key) const;
3638
};
3739

3840
} // namespace secret

0 commit comments

Comments
 (0)