Skip to content

Schema - Fix crash when editing a field with a null description#4197

Merged
agalin920 merged 2 commits into
devfrom
fix/schema-null-description-crash-on-edit
Jul 9, 2026
Merged

Schema - Fix crash when editing a field with a null description#4197
agalin920 merged 2 commits into
devfrom
fix/schema-null-description-crash-on-edit

Conversation

@agalin920

@agalin920 agalin920 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Editing (or opening the Add/Edit modal for) a Schema field whose description is null crashes the app with:

TypeError: Cannot read properties of null (reading 'length')
    at getErrorMessage (src/apps/schema/src/app/utils/index.ts:114)
    at FieldFormProvider.tsx (validation effect)

description is null for legacy and API-created fields (the API only stores a string once one is entered). Most fields created before the description field existed have null here.

Root cause

PR #4127 ("Schema: Add frontend validation for description field length", merged to dev 2025-05-28, reached stable 2025-07-01) added maxLength: 500 and validate: ["length"] to the description config entry. That routed description's value through getErrorMessage for the first time. description is the only validated input whose hydrated form value can be null (name/label are required non-null; tooltip is explicitly coerced to ""). The length check called value.length without a null guard.

Fix

Guard the length check in getErrorMessage so a null/empty value is treated as no error. This is the actual crash site and protects any nullable validated field, not just description.

closes #4196

Fields whose description is null (legacy and API-created fields store it
as null) crashed the Add/Edit Field modal on open with "Cannot read
properties of null (reading 'length')". PR #4127 added a length
validation to the description field, routing its value through
getErrorMessage, which called value.length without guarding against null.
Guard the length check so a null value is treated as no error.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review: Schema - Fix crash when editing a field with a null description

Summary

This is a targeted, correct crash fix. The root cause analysis in the PR description is accurate: description is the only validated field whose hydrated form value can be null, and the getErrorMessage length-check added in #4127 hit null.length without a guard.

The added value && condition short-circuits safely — null, undefined, and "" all skip the length check, which is the right behavior for an optional field.


Issues

Minor — type annotation doesn't reflect reality

getErrorMessageProps types value as string | FieldSettingsOptions[], but the call site passes null via an as string cast (FieldFormProvider.tsx ~line 356):

newErrorsObj[inputName] = getErrorMessage({
  value: formData[inputName] as string,  // null slips through here
  ...
});

The as string cast suppresses TypeScript, so the compiler gives no signal that null is possible. The fix is safe as written, but updating the type to string | FieldSettingsOptions[] | null would make TypeScript catch any future callers that could pass null without a guard — and is a truer description of what the function now handles. See inline comment.


Observations

  • The value && guard also catches undefined, which is good defensive coverage even if it is not the immediate bug.
  • The array branch of getErrorMessage (lines 76–85) accesses innerValue.length without a null guard, but innerValue comes from toPairs() on object key-value pairs so null cannot appear there in practice — no action needed.
  • The PR notes a Cypress regression test will follow separately. Worth tracking; a spec that opens the edit modal for a field with a null description and asserts no crash would be a solid guard against regression.

Verdict

The fix is correct and appropriately scoped. Approve pending the type annotation note (can be addressed here or as a fast follow-up).

Comment thread src/apps/schema/src/app/utils/index.ts
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Code Review

Overview

Minimal, targeted fix for a crash introduced by PR #4127. When description is null (true for any field created before the description field existed, or created via API), the length validation in getErrorMessage called value.length without a null guard. The fix adds value && before the length check.

Correctness

The fix is correct and handles the crash site precisely. A null description with validate: ["length"] no longer throws. An empty string ("") is also guarded, though that case would not have crashed (0 > maxLength is always false).

One issue: type annotation does not include null

The value parameter in getErrorMessageProps is typed as string | FieldSettingsOptions[], but null is now a documented, expected input. The guard silences the runtime crash but does not update the type contract, so TypeScript will not warn future callers about passing un-null-checked strings. The type should be widened to string | null | FieldSettingsOptions[]. See inline comment.

Minor observations

  • The PR description notes that tooltip is coerced to "" at hydration time but description is not. A future cleanup could apply the same coercion to description in FieldFormProvider.tsx, eliminating the null upstream. Out of scope here, but worth a follow-up.
  • The array branch (lines 76–85) calls innerValue.length > maxLength without a null guard, but innerValue is always a string extracted via toPairs from a FieldSettingsOptions object, so it cannot be null in practice — no change needed there.

Summary

Approve with one non-blocking suggestion to widen the type annotation to match the actual input contract.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Overall Coverage

Lines Statements Branches Functions
59.92% 59.57% 54.64% 54.40%

Changed Files Coverage

File Lines Statements Branches Functions
src/apps/schema/src/app/utils/index.ts 98.27% 95.00% 87.50% 100.00%

@agalin920 agalin920 merged commit b7e24f2 into dev Jul 9, 2026
11 checks passed
@agalin920 agalin920 deleted the fix/schema-null-description-crash-on-edit branch July 9, 2026 05:13
@sentry

sentry Bot commented Jul 9, 2026

Copy link
Copy Markdown

Sentry Issue: MANAGER-UI-3DF

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Schema Fields - app crashing when model field description is null

3 participants