Skip to content

Conversation

@dionlow
Copy link
Contributor

@dionlow dionlow commented Oct 21, 2025

Summary

  • Fix config validation to not throw errors that could crash parent app
  • Update OAuth mutation validation to use promise rejection instead of throw
  • All validation errors now flow through error callback pattern

Changes

Config Validation (src/headless/config/types.ts)

Before:

export function toCreateConfigContent(config): ConfigContent {
  if (!isValidCreateConfig(config)) {
    throw new Error("Config must have a provider field for creation");
  }
  return config;
}

After:

export function toCreateConfigContent(config): { data?: ConfigContent; error?: Error } {
  if (!isValidCreateConfig(config)) {
    return { error: new Error("Config must have a provider field for creation") };
  }
  return { data: config };
}

useCreateInstallation Hook

  • Now checks configResult.error before using config
  • Calls onError callback with validation error instead of letting exception bubble up

useUpdateOauthConnectMutation

  • Changed from throw new Error(...) to Promise.reject(new Error(...))
  • More explicit about returning rejected promise for React Query to handle

Why

During our error handling audit, we found that toCreateConfigContent was throwing synchronously during user actions (when createInstallation is called). This would bypass the error handling improvements from PR #1387 and could still crash the parent app.

Similarly, the OAuth mutation was throwing inside mutationFn which, while caught by React Query, is better expressed as an explicit Promise rejection.

Related PRs

Test plan

  • Verify createInstallation calls onError when config is missing provider field
  • Verify parent app doesn't crash when invalid config is provided
  • Verify useUpdateOauthConnectMutation properly rejects when required fields are missing
  • Verify error messages are surfaced correctly through callbacks
  • Verify build passes with no TypeScript errors

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings October 21, 2025 21:31
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 refactors error handling in configuration validation and OAuth mutations to prevent synchronous errors from crashing the parent application. Previously, toCreateConfigContent would throw errors during user actions, bypassing React Query's error handling. The changes convert these throwing functions to return result objects with explicit error fields, ensuring all errors flow through the established callback pattern.

Key Changes:

  • Modified toCreateConfigContent to return a result object ({ data?, error? }) instead of throwing
  • Updated useCreateInstallation to check for validation errors and route them through the onError callback
  • Changed useUpdateOauthConnectMutation to return rejected promises instead of throwing inside mutationFn

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/headless/config/types.ts Changed toCreateConfigContent return type from throwing to result object pattern
src/headless/installation/useCreateInstallation.ts Added validation check for config errors before creating installation request
src/hooks/mutation/useUpdateOauthConnectMutation.ts Replaced throw with Promise.reject for validation errors

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Dion Low and others added 3 commits October 23, 2025 12:19
Replace throwing errors with safe error handling in config validation and OAuth mutations.

Changes:
- Refactored toCreateConfigContent to return result object with data or error instead of throwing
- Updated useCreateInstallation to handle config validation errors via onError callback
- Changed useUpdateOauthConnectMutation to use Promise.reject instead of throw for clearer error handling

These validation errors now flow through the error callback pattern, preventing parent app crashes when:
- Config is missing required provider field
- OAuth update is called without required project ID or connection ID

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@dionlow dionlow force-pushed the fix/config-validation-error-handling branch from a4878ff to cc4bb7f Compare October 23, 2025 19:19
@dionlow dionlow requested review from a team and AmandaYao00 and removed request for a team October 23, 2025 19:25
@dionlow dionlow merged commit e1d7c33 into main Oct 30, 2025
4 checks passed
@dionlow dionlow deleted the fix/config-validation-error-handling branch October 30, 2025 21:31
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.

3 participants