Skip to content

Commit b0a737b

Browse files
authored
Merge pull request #420 from tisnik/lcore-463-missing-error-handling
LCORE-463: Added missing error handling
2 parents da067bc + ffb1c29 commit b0a737b

File tree

2 files changed

+41
-2
lines changed

2 files changed

+41
-2
lines changed

src/app/endpoints/feedback.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,12 @@ def store_feedback(user_id: str, feedback: dict) -> None:
145145

146146
# stores feedback in a file under unique uuid
147147
feedback_file_path = storage_path / f"{get_suid()}.json"
148-
with open(feedback_file_path, "w", encoding="utf-8") as feedback_file:
149-
json.dump(data_to_store, feedback_file)
148+
try:
149+
with open(feedback_file_path, "w", encoding="utf-8") as feedback_file:
150+
json.dump(data_to_store, feedback_file)
151+
except (OSError, IOError) as e:
152+
logger.error("Failed to store feedback at %s: %s", feedback_file_path, e)
153+
raise
150154

151155
logger.info("Feedback stored successfully at %s", feedback_file_path)
152156

tests/unit/app/endpoints/test_feedback.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,41 @@ def test_store_feedback(mocker, feedback_request_data):
154154
mock_json.dump.assert_called_once_with(expected_data, mocker.ANY)
155155

156156

157+
@pytest.mark.parametrize(
158+
"feedback_request_data",
159+
[
160+
{
161+
"conversation_id": "12345678-abcd-0000-0123-456789abcdef",
162+
"user_question": "What is OpenStack?",
163+
"llm_response": "It's some cloud thing.",
164+
"user_feedback": "This response is not helpful!",
165+
"sentiment": -1,
166+
},
167+
{
168+
"conversation_id": "12345678-abcd-0000-0123-456789abcdef",
169+
"user_question": "What is Kubernetes?",
170+
"llm_response": "K8s.",
171+
"sentiment": -1,
172+
"categories": ["incorrect", "not_relevant", "incomplete"],
173+
},
174+
],
175+
ids=["negative_text_feedback", "negative_feedback_with_categories"],
176+
)
177+
def test_store_feedback_on_io_error(mocker, feedback_request_data):
178+
"""Test the OSError and IOError handlings during feedback storage."""
179+
180+
# non-writable path
181+
# avoid touching the real filesystem; simulate a permission error on open
182+
configuration.user_data_collection_configuration.feedback_storage = "fake-path"
183+
mocker.patch("app.endpoints.feedback.Path", return_value=mocker.MagicMock())
184+
mocker.patch("builtins.open", side_effect=PermissionError("EACCES"))
185+
186+
user_id = "test_user_id"
187+
188+
with pytest.raises(OSError, match="EACCES"):
189+
store_feedback(user_id, feedback_request_data)
190+
191+
157192
def test_feedback_status():
158193
"""Test that feedback_status returns the correct status response."""
159194
configuration.user_data_collection_configuration.feedback_enabled = True

0 commit comments

Comments
 (0)