POST volume: return 422 when invalid node ID is passed - #1083
Open
boddumanohar wants to merge 2 commits into
Open
POST volume: return 422 when invalid node ID is passed#1083boddumanohar wants to merge 2 commits into
boddumanohar wants to merge 2 commits into
Conversation
mxsrc
requested changes
Jun 8, 2026
mxsrc
left a comment
mxsrc
left a comment
Collaborator
There was a problem hiding this comment.
This is not a validation error, the format of the value you passed in is a correct ID, just that there is no node with that ID. This should yield a 404. The problem is, that we can't simply return a 404 on any error from the backend function, because there may be other failures. To get a proper handling of this, there are two options:
- Look up the storage node before calling the add-lvol function to confirm it exists, or introduce an existence check into the database
- Return meaningful errors from the lvol_controller that can be mapped to HTTP codes.
add_lvol_ha now raises KeyError for an unresolvable host node instead of returning it as a generic (False, error) tuple, so the API layer can tell a missing-resource lookup apart from an ordinary validation failure and map it to 404 instead of 422.
boddumanohar
force-pushed
the
raise-404-error-on-node-not-found
branch
from
August 3, 2026 08:13
4448f81 to
5934b64
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves error handling for the v2 “create volume” endpoint so that invalid storage-node-id / host_id inputs don’t surface as uncaught exceptions (500), and instead return a client-meaningful HTTP error.
Changes:
- Map volume-create failures to HTTP errors (notably 422 for unprocessable input).
- Add special handling for missing/invalid host node IDs during volume create.
- Add a unit test covering the missing host node scenario.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
simplyblock_web/api/v2/cluster/storage_pool/volume/__init__.py |
Converts controller failures into HTTP responses for the v2 POST volume endpoint. |
simplyblock_core/controllers/lvol_controller.py |
Alters missing-node behavior in add_lvol_ha (currently by raising). |
tests/unit/web/api/v2/test_volume_endpoints.py |
Adds a regression test for missing/invalid host_id during volume creation. |
Suppressed comments (2)
simplyblock_web/api/v2/cluster/storage_pool/volume/init.py:119
- Use identity comparison for the sentinel False return and prefer HTTPException keyword args for clarity. Using
== Falsecan misclassify other falsy IDs (e.g., 0 or "") if they ever occur.
if volume_id_or_false == False: # noqa
raise HTTPException(422, error)
tests/unit/web/api/v2/test_volume_endpoints.py:89
- Assertion expects 404, but the PR title/description indicate invalid storage-node-id should result in 422 (unprocessable entity) rather than not-found.
assert response.status_code == 404
| host_node = nodes[0] | ||
| else: | ||
| return False, f"Can not find storage node: {host_id_or_name}" | ||
| raise KeyError(f"Can not find storage node: {host_id_or_name}") |
Comment on lines
+104
to
+105
| except KeyError as e: | ||
| raise HTTPException(404, str(e)) |
| ) | ||
| lvol_controller.add_lvol_ha.assert_not_called() | ||
|
|
||
| def test_missing_host_node_returns_404(self, client, db, pool, lvol_controller): |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When I passed incorrect storage-node-id as a part of volume create, it failed with 500 error. So handling this gracefully.