Skip to content

fix(core): strip trailing periods from error URLs #28069

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

Closed
aniruddhaadak80 wants to merge 1 commit into from
+34 −3
Closed

fix(core): strip trailing periods from error URLs #28069

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 24 additions & 0 deletions packages/core/src/utils/errors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,30 @@ describe('getErrorMessage', () => {
};
expect(getErrorMessage(error)).toBe('Bad Request Message');
});

it('should remove a trailing period from URLs in error messages', () => {
const error = new Error('Migrate to https://antigravity.google.');

expect(getErrorMessage(error)).toBe(
'Migrate to https://antigravity.google',
);
});

it('should preserve periods that do not trail a URL', () => {
expect(getErrorMessage(new Error('Authentication failed.'))).toBe(
'Authentication failed.',
);
});
Comment on lines +51 to +63

@gemini-code-assist gemini-code-assist Bot Jun 21, 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.

high

Add comprehensive test cases to verify the behavior of URLs wrapped in parentheses or brackets (both with periods inside and outside the brackets/parentheses) to ensure the regex handles these edge cases correctly and doesn't swallow closing brackets.

  it('should remove a trailing period from URLs in error messages', () => {
    const error = new Error('Migrate to https://antigravity.google.');

    expect(getErrorMessage(error)).toBe(
      'Migrate to https://antigravity.google',
    );
  });

  it('should handle URLs wrapped in parentheses or brackets correctly', () => {
    expect(getErrorMessage(new Error('Go to (https://google.com.)'))).toBe(
      'Go to (https://google.com)',
    );
    expect(getErrorMessage(new Error('Go to (https://google.com).'))).toBe(
      'Go to (https://google.com).',
    );
    expect(getErrorMessage(new Error('Go to [https://google.com.]'))).toBe(
      'Go to [https://google.com]',
    );
    expect(getErrorMessage(new Error('Go to [https://google.com].'))).toBe(
      'Go to [https://google.com].',
    );
  });

  it('should preserve periods that do not trail a URL', () => {
    expect(getErrorMessage(new Error('Authentication failed.'))).toBe(
      'Authentication failed.',
    );
  });


it.each([null, undefined])(
'should handle an Error with a nullish message',
(message) => {
const error = new Error('original');
Object.defineProperty(error, 'message', { value: message });

expect(getErrorMessage(error)).toBe('');
},
);
});

describe('isAbortError', () => {
Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/utils/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,16 @@ export function isAbortError(error: unknown): boolean {
return error instanceof Error && error.name === 'AbortError';
}

function stripTrailingPeriodsFromUrls(
message: string | null | undefined,
): string {
return message?.replace(/(https?:\/\/\S+)\.(?=\s|$)/g, '$1') ?? '';

@gemini-code-assist gemini-code-assist Bot Jun 21, 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.

high

The current regular expression /(https?:\/\/\S+)\.(?=\s|$)/g has a correctness issue when URLs are wrapped in parentheses or brackets, such as (https://google.com). or [https://google.com]..

Because \S+ is greedy and matches closing brackets/parentheses, it will swallow the closing parenthesis/bracket into the URL group $1 and strip the trailing period, resulting in "Go to (https://google.com)" (where the URL now incorrectly ends with a parenthesis). Conversely, for a URL like (https://google.com.) where the period is inside the parenthesis, the regex fails to match because the period is followed by ) (which is not whitespace or end of string).

We can make the regex more robust by:

  1. Excluding closing brackets/parentheses from the URL match group using [^\s)\]}]+.
  2. Allowing optional closing brackets/parentheses in the lookahead assertion: (?=[)\]}]?(?:\s|$)).

This ensures that periods outside parentheses are preserved (without swallowing the parenthesis), and periods inside parentheses are correctly stripped.

Suggested change
return message?.replace(/(https?:\/\/\S+)\.(?=\s|$)/g, '$1') ?? '';
return message?.replace(/(https?:\/\/[^\s)\]}]+)\.(?=[)\]}]?(?:\s|$))/g, '$1') ?? '';

}

export function getErrorMessage(error: unknown): string {
const friendlyError = toFriendlyError(error);
if (friendlyError instanceof Error) {
return friendlyError.message;
return stripTrailingPeriodsFromUrls(friendlyError.message);
}
if (
typeof friendlyError === 'object' &&
Expand All @@ -45,10 +51,11 @@ export function getErrorMessage(error: unknown): string {
typeof (friendlyError as { message: unknown }).message === 'string'
) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
return (friendlyError as { message: string }).message;
const message = (friendlyError as { message: string }).message;
return stripTrailingPeriodsFromUrls(message);
}
try {
return String(friendlyError);
return stripTrailingPeriodsFromUrls(String(friendlyError));
} catch {
return 'Failed to get error details';
}
Expand Down
Loading