EnxDev commented on code in PR #43459:
URL: https://github.com/apache/superset/pull/43459#discussion_r3880025250
##########
superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts:
##########
@@ -126,13 +126,17 @@ export function parseErrorJson(responseJson: JsonObject):
ClientErrorObject {
error.error = error.description = error.errors[0].message;
error.link = error.errors[0]?.extra?.link;
}
- // Marshmallow field validation returns the error message in the format
- // of { message: { field1: [msg1, msg2], field2: [msg], } }
+ // Marshmallow field validation returns { field1: [msg1, msg2], ... },
+ // but some validators attach a bare string instead of a list.
Review Comment:
Fixed the root cause at commands/dataset/update.py (both
_validate_expressions:386 and _validate_fetch_values_predicate:414): the
ValidationError message is now wrapped in a list ([message]) explicitly, so the
422 response keeps its documented {field: [msg]} shape regardless of whether
the underlying message is a str or a LazyString (marshmallow's own
isinstance(message, (str, bytes)) check doesn't cover LazyString, which is what
caused the bare-string storage). Added a regression assertion in update_test.py
checking messages stays a list.
##########
superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts:
##########
@@ -126,13 +126,17 @@ export function parseErrorJson(responseJson: JsonObject):
ClientErrorObject {
error.error = error.description = error.errors[0].message;
error.link = error.errors[0]?.extra?.link;
}
- // Marshmallow field validation returns the error message in the format
- // of { message: { field1: [msg1, msg2], field2: [msg], } }
+ // Marshmallow field validation returns { field1: [msg1, msg2], ... },
+ // but some validators attach a bare string instead of a list.
if (!error.error && error.message) {
if (typeof error.message === 'object') {
+ const firstFieldMessage = Object.values(
+ error.message as Record<string, string[] | string>,
Review Comment:
Checked the nested-object case ({"recipients": {"0": {"type": [...]}}}) —
confirmed it's already handled safely by the getFirstValidationError helper
that landed independently on master via
https://github.com/apache/superset/pull/43631 (merged into this branch): a
non-string/non-array field value returns undefined and falls back to t('Invalid
input') rather than leaking an object into error.error. Verified with a scratch
test. Left out of scope for this PR since it's pre-existing generic-fallback
behavior, not a regression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
