Skip to content

Conversation

@KyleTryon
Copy link
Contributor

This pull request focuses on improving error handling and type safety across several API routers, particularly for database operations that return arrays, by adding checks for missing results and providing more robust handling of optional values. It also standardizes pagination parameter handling in the articles router and makes minor improvements for stricter TypeScript usage.

Error Handling Improvements:

  • Added checks after database insert/update operations in categories, feeds, and subscriptions routers to throw explicit errors (using TRPCError or Error) if expected records are not returned, preventing silent failures and improving reliability. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]

  • Updated code to use non-null assertions (!) or explicit checks when accessing the first element of database result arrays, ensuring that code does not proceed with undefined values. [1] [2] [3] [4] [5]

Pagination and Query Parameter Consistency:

  • Standardized extraction and usage of pagination parameters (limit, offset, cursor) in the articles router to ensure defaults are respected and logic is consistent throughout, addressing TypeScript/Zod default handling issues. [1] [2] [3] [4] [5] [6] [7] [8]

TypeScript Strictness and Code Quality:

  • Added non-null assertions and improved destructuring in multiple places to satisfy stricter TypeScript checks and avoid potential runtime errors, especially for array accesses and optional chaining. [1] [2] [3] [4] [5]

Other Minor Improvements:

  • Improved parsing of HTTP status codes from error messages using optional chaining for increased robustness in error handling logic. [1] [2]
  • Updated CLI output in create-admin.ts to handle missing plan field gracefully.

KyleTryon and others added 3 commits December 9, 2025 23:32
Add 5 strict TypeScript compiler options to improve type safety:
- noUncheckedIndexedAccess: Ensures array access returns T | undefined
- useUnknownInCatchVariables: Catch variables typed as unknown
- allowUnreachableCode: Disallow unreachable code
- allowUnusedLabels: Disallow unused labels
- noImplicitOverride: Require explicit override keyword

Applied across all packages (api, app, tricorder) for consistent
type safety standards.

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

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fix ~100 type errors across API routers to comply with stricter
TypeScript configuration:

- Add non-null assertions after explicit runtime checks
- Use optional chaining for regex match groups
- Add explicit null checks for database .returning() calls
- Extract pagination params to preserve Zod default values
- Fix array access patterns with length guards

All changes maintain runtime safety while satisfying strict type
checking. No behavior changes.

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

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Reduce code duplication in admin.ts by extracting reusable components:

- Extract blockedDomainReasonSchema (used 8 times, saves ~60 lines)
- Extract planOutputSchema (used 2 times, saves ~12 lines)
- Extract globalSettingsOutputSchema
- Add transformAdminUser() helper (saves ~60 lines)
- Add formatGlobalSettings() helper (saves ~13 lines)

Net reduction: 56 lines removed (278 deleted, 222 added)
All tests passing, no behavior changes.

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

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@KyleTryon KyleTryon requested a review from Copilot December 10, 2025 04:36
@KyleTryon KyleTryon merged commit a958621 into main Dec 10, 2025
24 checks passed
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 pull request enhances TypeScript strictness by enabling several compiler options (noUncheckedIndexedAccess, useUnknownInCatchVariables, allowUnreachableCode: false, allowUnusedLabels: false, noImplicitOverride) across three packages (api, app, tricorder). The PR adds defensive null checks and non-null assertions throughout the codebase to satisfy stricter type checking, particularly for array access operations and database query results.

Key changes:

  • Enhanced TypeScript compiler options for improved type safety across all packages
  • Added explicit error handling after database insert/update operations to prevent silent failures
  • Standardized pagination parameter extraction in articles and admin routers to preserve Zod defaults

Reviewed changes

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

Show a summary per file
File Description
packages/tricorder/tsconfig.json Enables stricter TypeScript options for better type safety
packages/app/tsconfig.app.json Enables stricter TypeScript options for better type safety
packages/api/tsconfig.json Enables stricter TypeScript options for better type safety
packages/tricorder/src/services/apple-discovery.ts Adds defensive check for empty results array and uses optional chaining for array access
packages/api/src/services/feed-discovery/apple-discovery.ts Adds defensive check for empty results array and uses optional chaining for array access
packages/api/src/utils/admin-metrics.ts Adds non-null assertions for ISO string split operations
packages/api/src/types/validators.ts Removes redundant .string() calls and adds defensive domain check in email validator
packages/api/src/routers/userSettings.ts Adds null checks after database insert/update operations
packages/api/src/routers/subscriptions.ts Adds non-null assertions for array access and explicit error handling after insert operations
packages/api/src/routers/feeds.ts Adds explicit error handling after database insert/update operations
packages/api/src/routers/categories.ts Adds explicit error handling after database insert/update operations
packages/api/src/routers/articles.ts Extracts pagination parameters to preserve defaults and adds non-null assertion for query result
packages/api/src/routers/admin.ts Refactors duplicate code into helper functions and extracts input parameters for type safety
packages/api/src/hono/app.ts Adds non-null assertion for CORS origins array access
packages/api/src/cli/create-admin.ts Uses optional chaining for safer access to plan field

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

Comment on lines 110 to 120
// Update usage stats
await incrementCategoryCount(ctx.db, userId);

if (!newCategory[0]) {
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "Failed to create category",
});
}

return newCategory[0];
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The usage stats are being incremented before verifying that the category was successfully created. If the insert operation returns an empty array (line 113), the usage count will have already been incorrectly incremented at line 111. Consider moving the incrementCategoryCount call to after the successful creation check (after line 118) to ensure stats are only updated when the category is actually created.

Copilot uses AI. Check for mistakes.
Comment on lines +169 to +174
// Check if podcast exists in results
if (!podcast) {
span.setStatus({ code: 2, message: "No podcast in results" });
return [];
}

Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

This check is redundant with the earlier check at lines 153-157. If data.results.length === 0 is true at line 156, the function would have already returned, so data.results[0] can never be undefined at this point. While the defensive check helps satisfy TypeScript's noUncheckedIndexedAccess, it represents unreachable code. Consider removing this check or adding a comment explaining it's for TypeScript type narrowing only.

Suggested change
// Check if podcast exists in results
if (!podcast) {
span.setStatus({ code: 2, message: "No podcast in results" });
return [];
}
// At this point, podcast is guaranteed to exist due to earlier checks.

Copilot uses AI. Check for mistakes.
Comment on lines +170 to +177
// Check if podcast exists in results
if (!podcast) {
context.telemetry?.addBreadcrumb?.({
message: "No results found for podcast ID",
data: { podcast_id: podcastId },
});
return [];
}
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

This check is redundant with the earlier check at lines 156-165. If data.results.length === 0 is true at line 159, the function would have already returned, so data.results[0] can never be undefined at this point. While the defensive check helps satisfy TypeScript's noUncheckedIndexedAccess, it represents unreachable code. Consider removing this check or adding a comment explaining it's for TypeScript type narrowing only.

Suggested change
// Check if podcast exists in results
if (!podcast) {
context.telemetry?.addBreadcrumb?.({
message: "No results found for podcast ID",
data: { podcast_id: podcastId },
});
return [];
}
// At this point, podcast is guaranteed to be defined due to the earlier length check.

Copilot uses AI. Check for mistakes.
@KyleTryon KyleTryon deleted the feat/enhanced-typescript-options branch January 8, 2026 19:46
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.

2 participants