[26.0] Raise error when API client sends invalid parameter keys#22277
Conversation
The Sentry crash (issue galaxyproject#22132) was triggered by a Python Requests 2.32 client sending an invalid payload. In this case the input was ```json { "in": "custom", "in|custom|mtx": { "id": "f9cad7b01a472135e9dd0bb539e2d0de", "src": "hda" }, "in|custom|obs": { "id": "f9cad7b01a472135068f117b3249ac19", "src": "hda" }, "in|custom|var": { "id": "f9cad7b01a472135e497326e87a7fa84", "src": "hda" } } ``` where `in` is a conditional and `adata_format` is the tester select that you can in fact set to `custom`. Replace the opaque AttributeError with a RequestParameterInvalidException that tells the caller exactly what went wrong and how to fix their parameter naming. Fixes galaxyproject#22132
mvdbeek
force-pushed
the
raise-exception-invalid-param
branch
from
d9b12b8 to
ec0df9d
Compare
github-actions
Bot
changed the title
| f"but '{input_name}' was already assigned the plain value {subdict}. " | ||
| f"Use the full prefixed parameter name (e.g. '<conditional_name>|<input_name>') " | ||
| f"instead of the unprefixed input name." | ||
| ) |
There was a problem hiding this comment.
It says in the comment above this could be a section and not just a conditional. Also - where is the evidence that this is how subdict became not a subdict. It feel likes Claude walking one potential value through the logic and hitting one kind of error and then generalizing. I'm fine with replacing the error type but this error message feels wrong or unsupported to me. Maybe if I spend more time with it - I would feel differently though.
There was a problem hiding this comment.
It says in the comment above this could be a section and not just a conditional. Also - where is the evidence that this is how subdict became not a subdict.
The payload is in the PR description, I deleted the unit test that seemed like nonsense. I did struggle finding a better message here. Yes, it doesn't need to be a conditional, any invalid parameter that conflicts with a valid input is going to fail this.
There was a problem hiding this comment.
I assume validating against the schema is going to solve that problem entirely so I thought we can let that one slide 😆
There was a problem hiding this comment.
Scene cut to how that work is going 😭😭😓...
There was a problem hiding this comment.
I mean i don't have all the context but this doesn't seem to be entirely right ?
There was a problem hiding this comment.
The context is totally unrelated I just wanted to scream into the void - it is workflow state - how we do the maybe it is encoded or not thing - I don't know how it could be consistent but hopefully I'll prove myself wrong.
github-project-automation
Bot
moved this from Needs Review
to Done
in Galaxy Dev - weeklies
3 participants
The Sentry crash (issue #22132) was triggered by a Python Requests 2.32
client sending an invalid payload.
In this case the input was
{ "in": "custom", "in|custom|mtx": { "id": "f9cad7b01a472135e9dd0bb539e2d0de", "src": "hda" }, "in|custom|obs": { "id": "f9cad7b01a472135068f117b3249ac19", "src": "hda" }, "in|custom|var": { "id": "f9cad7b01a472135e497326e87a7fa84", "src": "hda" } }where
inis a conditional andadata_formatis the tester select thatyou can in fact set to
custom.Replace the opaque AttributeError with a RequestParameterInvalidException
that tells the caller exactly what went wrong and how to fix their
parameter naming.
Fixes #22132
Not adding tests here, I suppose that'll be fixed and covered once we validate against the tool state.
How to test the changes?
(Select all options that apply)
License