Skip to content

Comments

fix(VCST-4224): aborted mutations safari error notifications#2058

Merged
ivan-kalachikov merged 2 commits intodevfrom
fix/VCST-4224-aborted-errors
Nov 17, 2025
Merged

fix(VCST-4224): aborted mutations safari error notifications#2058
ivan-kalachikov merged 2 commits intodevfrom
fix/VCST-4224-aborted-errors

Conversation

@ivan-kalachikov
Copy link
Contributor

@ivan-kalachikov ivan-kalachikov commented Nov 17, 2025

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where aborted GraphQL mutations in Safari were incorrectly triggering error notifications. The fix enhances the abort detection logic to handle Safari's specific implementation of AbortError.

  • Enhanced abort error detection to recognize Safari's AbortError format by checking both the error name property and message string
  • Refactored to extract abort message into a variable to avoid redundant toString() calls
Comments suppressed due to low confidence (1)

client-app/core/api/graphql/utils.ts:39

  • This critical error handling function lacks test coverage. Given that:
  • Other utility functions in the codebase have comprehensive test coverage (e.g., core/utilities/, core/composables/)
  • This function handles important error routing logic that affects user experience
  • This PR is specifically fixing a Safari-specific bug with AbortError detection

Consider adding tests to cover:

  • The explicit abort reason check
  • Safari's AbortError name property
  • AbortError in the message string
  • Regular network errors that should trigger ServerError.Unhandled
  • GraphQL errors with different error codes
export function toServerError(
  networkError: NetworkError | undefined,
  graphQLErrors: ReadonlyArray<GraphQLFormattedError> | undefined,
): ServerError | undefined {
  const abortMessage = networkError?.toString() ?? "";
  const isExplicitlyAborted =
    abortMessage.includes(AbortReason.Explicit) ||
    abortMessage.includes("AbortError") ||
    networkError?.name === "AbortError";

  if ((networkError && !isExplicitlyAborted) || hasErrorCode(graphQLErrors, GraphQLErrorCode.Unhandled)) {
    return ServerError.Unhandled;
  }

  if (hasErrorCode(graphQLErrors, GraphQLErrorCode.Unauthorized)) {
    return ServerError.Unauthorized;
  }

  if (hasErrorCode(graphQLErrors, GraphQLErrorCode.Forbidden)) {
    return ServerError.Forbidden;
  }
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sonarqubecloud
Copy link

@ivan-kalachikov ivan-kalachikov merged commit 3f0a45a into dev Nov 17, 2025
9 checks passed
@ivan-kalachikov ivan-kalachikov deleted the fix/VCST-4224-aborted-errors branch November 17, 2025 11:07
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.

4 participants