Compare commits

...
Author SHA1 Message Date
Sonarly Claude Code d2746d2778 UpdateWorkflowVersionStep returns unvalidated position object causing GraphQL non-nullable field error
https://sonarly.com/issue/4115?type=bug

The UpdateWorkflowVersionStep mutation returns a step object with an invalid `position` field (non-null object with null/undefined `x` and `y`), causing GraphQL to throw a non-nullable field violation on `WorkflowStepPosition.x`.

Fix: Added a `sanitizeStepPosition` private method and position normalization logic to `WorkflowVersionStepUpdateWorkspaceService.updateWorkflowVersionStep()`.

**What changed:**

1. **Position fallback** (line 76-78): After obtaining `updatedStep` from either the type-changed or settings-only path, the code now resolves position using `updatedStep.position ?? existingStep.position`. This means if the client input doesn't include position (common for settings-only updates), the existing position from the database is preserved rather than lost.

2. **Position sanitization** (lines 108-120): A new `sanitizeStepPosition` method validates that if position is defined, both `x` and `y` must be actual numbers (`typeof === 'number'`). If either is missing, null, undefined, or non-numeric, the entire position is set to `undefined` — which GraphQL correctly resolves as `null` for the nullable `position` field on `WorkflowActionDTO`. This prevents the `Cannot return null for non-nullable field WorkflowStepPosition.x` error.

3. **Normalized step used everywhere** (lines 80-105): The `normalizedUpdatedStep` (with sanitized position) is used both for saving to the database and as the GraphQL return value, ensuring consistency between what's persisted and what's returned.

The fix is applied at the single return point of the public `updateWorkflowVersionStep` method, covering both the type-changed path (where `existingStep.position` from DB could be corrupted) and the settings-only path (where client input typically omits position).
2026-03-06 10:55:27 +00:00
Sonarly Claude Code cb05c538b3 chore: improve monitoring for Missing exception filter causes expected FORBIDDEN
Registered the new `WorkflowQueryValidationGraphqlApiExceptionFilter` on two resolvers that call code paths throwing `WorkflowQueryValidationException`:

1. **`workflow-version-step.resolver.ts`** — handles `UpdateWorkflowVersionStep` (the mutation in this Sentry error)
2. **`workflow-version-edge.resolver.ts`** — handles edge operations that also call `getValidatedDraftWorkflowVersion`

With the filter registered, `WorkflowQueryValidationException` is now converted to a `ForbiddenError` (`BaseGraphQLError` with `ErrorCode.FORBIDDEN`). The `shouldCaptureException` function in the GraphQL error handler hook will now correctly filter this out (since `FORBIDDEN` is in `graphQLErrorCodesToFilter`), eliminating the Sentry noise from this expected validation error.
2026-03-06 10:54:05 +00:00
Sonarly Claude Code 0d151571c8 Missing exception filter causes expected FORBIDDEN validation error to be captured by Sentry
https://sonarly.com/issue/3892?type=bug

The `WorkflowQueryValidationException` thrown by `assertWorkflowVersionIsDraft` has no NestJS exception filter to convert it to a proper GraphQL error, causing it to bypass Sentry's error filtering and be incorrectly reported as an unhandled error. The underlying trigger is a frontend race condition where stale React state sends a step update mutation for a workflow version that is no longer in DRAFT status.

Fix: Created a new `WorkflowQueryValidationGraphqlApiExceptionFilter` that catches `WorkflowQueryValidationException` and converts it to a proper `ForbiddenError` (a `BaseGraphQLError`). This follows the exact same pattern as the three existing workflow exception filters in the codebase.

**Why this fixes the issue:** The `WorkflowQueryValidationException` (thrown by `assertWorkflowVersionIsDraft`) was not caught by any exception filter on the workflow resolvers. Without a filter, the exception bypassed the `shouldCaptureException` logic in the global error handler (which only filters `HttpException`, `GraphQLError`, and `BaseGraphQLError` types), causing every instance to be captured by Sentry as if it were an unhandled server error.

The new filter converts `WorkflowQueryValidationException` → `ForbiddenError` (which extends `BaseGraphQLError` with `ErrorCode.FORBIDDEN`). Since `ErrorCode.FORBIDDEN` is in `graphQLErrorCodesToFilter`, the `shouldCaptureException` function will now correctly return `false`, preventing Sentry capture.

The user-facing behavior remains correct: the `ForbiddenError` is still returned as a GraphQL error with the user-friendly message, so the frontend still receives and handles it properly.

**Note:** The underlying frontend race condition (stale React closure in `useGetUpdatableWorkflowVersionOrThrow`) still exists. This fix correctly classifies the exception as an expected validation error rather than masking it.
2026-03-06 10:54:05 +00:00
@@ -73,9 +73,18 @@ export class WorkflowVersionStepUpdateWorkspaceService {
additionalCreatedSteps: undefined,
};
const resolvedPosition = this.sanitizeStepPosition(
updatedStep.position ?? existingStep.position,
);
const normalizedUpdatedStep = {
...updatedStep,
position: resolvedPosition,
};
const updatedSteps = workflowVersion.steps.map((existingStep) => {
if (existingStep.id === step.id) {
return updatedStep;
return normalizedUpdatedStep;
} else {
return existingStep;
}
@@ -93,7 +102,21 @@ export class WorkflowVersionStepUpdateWorkspaceService {
},
);
return updatedStep;
return normalizedUpdatedStep;
}
private sanitizeStepPosition(
position: WorkflowAction['position'],
): WorkflowAction['position'] {
if (
!isDefined(position) ||
typeof position.x !== 'number' ||
typeof position.y !== 'number'
) {
return undefined;
}
return position;
}
private async updateWorkflowVersionStepType({