Skip to content

Add type guard for Yup ValidationError before accessing inner property #1000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
levalleux-ludo merged 3 commits into from
Feb 13, 2026
Merged

Add type guard for Yup ValidationError before accessing inner property #1000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
73ecd91
Initial plan
Copilot Feb 13, 2026
24c9297
Add type checking for Yup ValidationError before accessing inner prop…
Copilot Feb 13, 2026
6107f4f
Fix spacing in function call
Copilot Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 38 additions & 7 deletions packages/core-sdk/src/offers/checkExchangePolicy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ export type CheckExchangePolicyRules = {
}[];
};

// Type guard to check if an error is a Yup ValidationError with inner errors
function isValidationErrorWithInner(
error: unknown
): error is { inner: Array<{ message: string; path?: string; value?: unknown }> } {
return (
typeof error === "object" &&
error !== null &&
"inner" in error &&
Array.isArray((error as { inner: unknown }).inner)
);
Comment on lines +71 to +77
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type guard's return type declares path?: string and value?: unknown as optional, but the CheckExchangePolicyResult type (lines 5-12) requires these properties to be non-optional (path: string and value: unknown). This type mismatch could lead to runtime errors when consumers expect these properties to always be present. Additionally, the type guard only validates that inner is an array but doesn't verify that the array elements have the required structure with a message property.

Suggested change
): error is { inner: Array<{ message: string; path?: string; value?: unknown }> } {
return (
typeof error === "object" &&
error !== null &&
"inner" in error &&
Array.isArray((error as { inner: unknown }).inner)
);
): error is { inner: Array<{ message: string; path: string; value: unknown }> } {
if (
typeof error !== "object" ||
error === null ||
!("inner" in error) ||
!Array.isArray((error as { inner: unknown }).inner)
) {
return false;
}
const { inner } = error as { inner: unknown[] };
return inner.every((item) => {
if (typeof item !== "object" || item === null) {
return false;
}
const candidate = item as {
message?: unknown;
path?: unknown;
value?: unknown;
};
return (
typeof candidate.message === "string" &&
typeof candidate.path === "string" &&
"value" in candidate
);
});

Copilot uses AI. Check for mistakes.
}

export function checkExchangePolicy(
offerData: OfferFieldsFragment,
rules: CheckExchangePolicyRules
Expand Down Expand Up @@ -109,10 +121,18 @@ export function checkExchangePolicy(
} catch (e) {
result = {
isValid: false,
errors:
e.inner?.map((error) => {
return { ...error };
}) || []
errors: isValidationErrorWithInner(e)
? e.inner.map((error) => {
return { ...error };
})
Comment on lines +125 to +127
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When spreading Yup ValidationError objects with the spread operator, additional properties beyond message, path, and value may be copied into the result. While this may not cause immediate issues, it could lead to unexpected behavior if the result is strictly validated elsewhere or if consumers rely on the type definition. Consider explicitly mapping only the expected properties to ensure the result structure exactly matches the CheckExchangePolicyResult type.

Suggested change
? e.inner.map((error) => {
return { ...error };
})
? e.inner.map((error) => ({
message: error.message,
path: error.path ?? "",
value: error.value
}))

Copilot uses AI. Check for mistakes.
: [
{
message:
e instanceof Error ? e.message : "Validation failed with unknown error",
path: "",
value: undefined
}
]
Comment on lines +128 to +135
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no test cases covering the new error handling path for non-Yup ValidationError scenarios. The fallback error handling (lines 128-135 and 171-180) that returns "Validation failed with unknown error" is not tested. Consider adding test cases to verify that errors without the inner property are handled correctly and return the expected error structure.

Copilot uses AI. Check for mistakes.
};
}
if (metadataType === "BUNDLE") {
Expand Down Expand Up @@ -144,9 +164,20 @@ export function checkExchangePolicy(
} catch (e) {
result.isValid = false;
result.errors = result.errors.concat(
e.inner?.map((error) => {
return { ...error };
}) || []
isValidationErrorWithInner(e)
? e.inner.map((error) => {
return { ...error };
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When spreading Yup ValidationError objects with the spread operator, additional properties beyond message, path, and value may be copied into the result. While this may not cause immediate issues, it could lead to unexpected behavior if the result is strictly validated elsewhere or if consumers rely on the type definition. Consider explicitly mapping only the expected properties to ensure the result structure exactly matches the CheckExchangePolicyResult type.

Suggested change
return { ...error };
return {
message: error.message,
path: error.path ?? "",
value: error.value
};

Copilot uses AI. Check for mistakes.
})
: [
{
message:
e instanceof Error
? e.message
: "Validation failed with unknown error",
path: "",
value: undefined
}
]
);
}
}
Expand Down
Loading