Skip to content

refactor(core): remove unsafe type assertions in error utils (Phase 1.1) #19750

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
mattKorwel merged 2 commits into from
Feb 21, 2026
Merged

refactor(core): remove unsafe type assertions in error utils (Phase 1.1) #19750

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c8d8ed
refactor(core): remove unsafe type assertions in error utils
mattKorwel Feb 20, 2026
419c81a
refactor(core): address code review feedback for type guards
mattKorwel Feb 20, 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
72 changes: 61 additions & 11 deletions packages/core/src/utils/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ interface GaxiosError {
};
}

function isGaxiosError(error: unknown): error is GaxiosError {
return (
typeof error === 'object' &&
error !== null &&
'response' in error &&
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
typeof (error as GaxiosError).response === 'object'
);
}
Comment on lines +13 to +21

@gemini-code-assist gemini-code-assist Bot Feb 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

The type guard isGaxiosError is not safe because typeof null is 'object'. If error.response is null, this check will incorrectly pass. The GaxiosError interface expects response to be an object with a data property or undefined, but not null. Additionally, the type assertion as GaxiosError is unsafe and can be avoided, which aligns with the goal of this PR.

Suggested change
function isGaxiosError(error: unknown): error is GaxiosError {
return (
typeof error === 'object' &&
error !== null &&
'response' in error &&
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
typeof (error as GaxiosError).response === 'object'
);
}
function isGaxiosError(error: unknown): error is GaxiosError {
return (
typeof error === 'object' &&
error !== null &&
'response' in error &&
typeof (error as { response: unknown }).response === 'object' &&
(error as { response: unknown }).response !== null
);
}


export function isNodeError(error: unknown): error is NodeJS.ErrnoException {
return error instanceof Error && 'code' in error;
}
Expand Down Expand Up @@ -105,11 +115,41 @@ interface ResponseData {
};
}

function isResponseData(data: unknown): data is ResponseData {
if (typeof data !== 'object' || data === null) {
return false;
}
const candidate = data as ResponseData;
if (!('error' in candidate)) {
return false;
}
const error = candidate.error;
if (typeof error !== 'object' || error === null) {
return false; // error property exists but is not an object (could be undefined, but we checked 'in')
}

// Optional properties check
if (
'code' in error &&
typeof error.code !== 'number' &&
error.code !== undefined
) {
return false;
}
if (
'message' in error &&
typeof error.message !== 'string' &&
error.message !== undefined
) {
return false;
}

return true;
}

export function toFriendlyError(error: unknown): unknown {
if (error && typeof error === 'object' && 'response' in error) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const gaxiosError = error as GaxiosError;
const data = parseResponseData(gaxiosError);
if (isGaxiosError(error)) {
const data = parseResponseData(error);
if (data && data.error && data.error.message && data.error.code) {
switch (data.error.code) {
case 400:
Expand All @@ -129,17 +169,20 @@ export function toFriendlyError(error: unknown): unknown {
}

function parseResponseData(error: GaxiosError): ResponseData | undefined {
let data = error.response?.data;
// Inexplicably, Gaxios sometimes doesn't JSONify the response data.
if (typeof error.response?.data === 'string') {
if (typeof data === 'string') {
try {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
return JSON.parse(error.response?.data) as ResponseData;
data = JSON.parse(data);
} catch {
return undefined;
}
}
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
return error.response?.data as ResponseData | undefined;

if (isResponseData(data)) {
return data;
}
return undefined;
}

/**
Expand All @@ -152,8 +195,15 @@ function parseResponseData(error: GaxiosError): ResponseData | undefined {
export function isAuthenticationError(error: unknown): boolean {
// Check for MCP SDK errors with code property
// (SseError and StreamableHTTPError both have numeric 'code' property)
if (error && typeof error === 'object' && 'code' in error) {
const errorCode = (error as { code: unknown }).code;
if (
error &&
typeof error === 'object' &&
'code' in error &&
typeof (error as { code: unknown }).code === 'number'
) {
// Safe access after check
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const errorCode = (error as { code: number }).code;
if (errorCode === 401) {
return true;
}
Expand Down
31 changes: 26 additions & 5 deletions packages/core/src/utils/googleErrors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,13 @@ export function parseGoogleApiError(error: unknown): GoogleApiError | null {
detailObj['@type'] = detailObj[typeKey];
delete detailObj[typeKey];
}
// We can just cast it; the consumer will have to switch on @type
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
details.push(detailObj as unknown as GoogleApiErrorDetail);
// Basic structural check before casting.
// Since the proto definitions are loose, we primarily rely on @type presence.
if (typeof detailObj['@type'] === 'string') {
// We can just cast it; the consumer will have to switch on @type
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
details.push(detailObj as unknown as GoogleApiErrorDetail);
}
}
}
}
Expand All @@ -223,6 +227,17 @@ export function parseGoogleApiError(error: unknown): GoogleApiError | null {
return null;
}

function isErrorShape(obj: unknown): obj is ErrorShape {
return (
typeof obj === 'object' &&
obj !== null &&
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
(('message' in obj && typeof (obj as ErrorShape).message === 'string') ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
('code' in obj && typeof (obj as ErrorShape).code === 'number'))
);
}
Comment on lines +230 to +238

@gemini-code-assist gemini-code-assist Bot Feb 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

The type guard isErrorShape uses an unsafe type assertion as ErrorShape which this pull request aims to remove. The assertions are suppressed with eslint comments, but they can be avoided altogether for a safer implementation.

You can make this type guard safer by using a more constrained type for the cast, based on what has already been verified, which also allows removing the eslint-disable comments.

function isErrorShape(obj: unknown): obj is ErrorShape {
  return (
    typeof obj === 'object' &&
    obj !== null &&
    (('message' in obj &&
      typeof (obj as { message: unknown }).message === 'string') ||
      ('code' in obj && typeof (obj as { code: unknown }).code === 'number'))
  );
}


function fromGaxiosError(errorObj: object): ErrorShape | undefined {
const gaxiosError = errorObj as {
response?: {
Expand Down Expand Up @@ -256,7 +271,10 @@ function fromGaxiosError(errorObj: object): ErrorShape | undefined {
if (typeof data === 'object' && data !== null) {
if ('error' in data) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
outerError = (data as { error: ErrorShape }).error;
const potentialError = (data as { error: unknown }).error;
if (isErrorShape(potentialError)) {
outerError = potentialError;
}
}
}
}
Expand Down Expand Up @@ -313,7 +331,10 @@ function fromApiError(errorObj: object): ErrorShape | undefined {
if (typeof data === 'object' && data !== null) {
if ('error' in data) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
outerError = (data as { error: ErrorShape }).error;
const potentialError = (data as { error: unknown }).error;
if (isErrorShape(potentialError)) {
outerError = potentialError;
}
}
}
}
Expand Down
Loading