fix(forms): propagate request field errors in autosave#113541
fix(forms): propagate request field errors in autosave#113541sentry-junior[bot] wants to merge 2 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 91572ba. Configure here.
| error instanceof RequestError | ||
| ? error | ||
| : ({[name]: {message: t('Failed to save')}} as never) | ||
| ); |
There was a problem hiding this comment.
Silent error when RequestError lacks matching field errors
High Severity
When a RequestError is thrown but its responseJSON doesn't contain a key matching the form's field name (e.g., a 500 error with {detail: "Internal server error"}, or a 403 with no field-level errors), setFieldErrors silently returns without setting any error. The user gets no feedback at all. Previously, every error produced a "Failed to save" message. The fallback generic message is needed when setFieldErrors extracts zero matching field errors from a RequestError.
Reviewed by Cursor Bugbot for commit 91572ba. Configure here.
|
|
||
| setFieldErrors( | ||
| formApi, | ||
| error instanceof RequestError | ||
| ? error | ||
| : ({[name]: {message: t('Failed to save')}} as never) | ||
| ); |
There was a problem hiding this comment.
Bug: The new error handling for RequestError can lead to silent failures. The setFieldErrors function won't show an error for network issues or API responses with non-field errors.
Severity: HIGH
Suggested Fix
After attempting to set errors from a RequestError, check if any errors were actually set. If no specific field errors could be mapped from the response, fall back to displaying a generic error message like t('Failed to save'). This ensures the user is always notified of a save failure, restoring the previous behavior for unhandled error types.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: static/app/components/core/form/autoSaveForm.tsx#L200-L206
Potential issue: In `autoSaveForm.tsx`, `RequestError` instances are now passed directly
to the `setFieldErrors` function. However, `setFieldErrors` only displays an error if
the `RequestError` contains a `responseJSON` with keys that match the form's field
names. This means that for common API failure scenarios, such as network errors, 500
responses without a JSON body, or responses with non-field errors (e.g., `{"detail":
"..."}`), no error message will be shown to the user. This creates a silent failure
where the user may believe their changes were saved when they were not, which is a
regression from the previous behavior that showed a generic failure message.
Did we get this right? 👍 / 👎 to inform future reviews.
|
I’ll tackle a proper fix with DE-1180 |


summary
RequestErrorinstances throughAutoSaveFormerror handling instead of always replacing them withFailed to savetesting
Refs DE-1181