Skip to content

fix: unify error handling in logs and functions/invoke#23

Merged
jwfing merged 4 commits intoInsForge:mainfrom
Abh1shxkk:fix/unified-error-handler
Mar 17, 2026
Merged

fix: unify error handling in logs and functions/invoke#23
jwfing merged 4 commits intoInsForge:mainfrom
Abh1shxkk:fix/unified-error-handler

Conversation

@Abh1shxkk
Copy link
Contributor

@Abh1shxkk Abh1shxkk commented Mar 15, 2026

Closes #18

Replace console.error + process.exit(1) with CLIError throws in logs.ts and functions/invoke.ts so all errors flow through the central handleError function.

Summary by CodeRabbit

  • Bug Fixes
    • Unified error handling so command failures now propagate through a single error pathway instead of terminating the process immediately.
    • Suppressed redundant inline HTTP error messages for cleaner output; non-JSON responses still display their text.
    • Invalid log sources and function deployment failures now surface via the unified error flow for a more consistent and catchable user experience.

@coderabbitai
Copy link

coderabbitai bot commented Mar 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 89454540-3957-41f6-b77b-56ebfffdad79

📥 Commits

Reviewing files that changed from the base of the PR and between 338dc8f and aac5e51.

📒 Files selected for processing (1)
  • src/commands/functions/invoke.ts

Walkthrough

Three CLI command modules switch from immediate process termination to throwing CLIError for unified error handling; invoke.ts also removes inline HTTP error logging to stderr. Error propagation now flows through the centralized exception handler.

Changes

Cohort / File(s) Summary
Functions: invoke
src/commands/functions/invoke.ts
Import CLIError; remove inline HTTP error logging to stderr; on HTTP error status throw CLIError instead of calling process.exit. Non-JSON success behavior unchanged.
Logs command
src/commands/logs.ts
Import CLIError; replace console.error + process.exit(1) on invalid log source with throwing CLIError, allowing centralized handler to manage termination.
Functions: deploy
src/commands/functions/deploy.ts
Replace process.exit(1) on deployment failure with throw new CLIError('Function deployment failed'); reporting flow adjusted to use exception path for failures.
Manifest
package.json
Small manifest edits (few lines changed) likely metadata or dependency adjustments accompanying code changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through code with careful cheer,

No sudden exits now appear,
CLIError gathers every fall,
One gentle path to catch them all,
I munched a carrot and gave a cheer.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: unifying error handling by replacing process.exit calls with CLIError throws in logs and functions/invoke files.
Linked Issues check ✅ Passed All coding requirements from issue #18 are met: console.error + process.exit(1) replaced with CLIError throws in logs.ts, functions/invoke.ts, and functions/deploy.ts, with errors routing through handleError.
Out of Scope Changes check ✅ Passed All changes are in-scope: modifications to logs.ts and functions/invoke.ts directly address issue #18, and functions/deploy.ts was added per reviewer request to maintain consistency.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can validate your CodeRabbit configuration file in your editor.

If your editor has YAML language server, you can enable auto-completion and validation by adding # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json at the top of your CodeRabbit configuration file.

@jwfing jwfing self-requested a review March 15, 2026 20:35
@jwfing
Copy link
Member

jwfing commented Mar 15, 2026

@Abh1shxkk Could you change other files too?, for example:

  • src/commands/functions/deploy.ts also has a bare process.exit(1) that could be converted.
    If we can unify the entire project in one PR, it's great for maintenance in the future. Thanks.

@Abh1shxkk
Copy link
Contributor Author

hi @jwfing done also fixed src/commands/functions/deploy.ts replaced the bare process.exit(1) with a CLIError throw so it flows through handleError. Checked all other files too, no other occurrences remain.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/commands/functions/deploy.ts`:
- Around line 82-86: The code double-reports failures because
reportCliUsage('cli.functions.deploy', !deployFailed) is called just before
throwing CLIError and then again in the catch; remove the pre-throw call and
ensure reportCliUsage is invoked once: call reportCliUsage(true) on the
successful path (when deployFailed is false) and let the catch block call
reportCliUsage(false) on errors. Update the logic around the deploy flow
(references: reportCliUsage, deployFailed, CLIError, handleError, json) so
failures are only reported in the catch and successes are reported once after
the try completes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f57285ee-4977-4553-9667-ebf29ea00a70

📥 Commits

Reviewing files that changed from the base of the PR and between 3c4057c and c30b75f.

📒 Files selected for processing (1)
  • src/commands/functions/deploy.ts

@Abh1shxkk
Copy link
Contributor Author

Hey @jwfing, addressed both review points:

  1. logs.tsconsole.error + process.exit(1) removed, replaced with throw new CLIError(...) so it flows through handleError.

  2. functions/invoke.ts — Added 'HTTP_ERROR' as the code to CLIError, so JSON output now correctly serializes as {"error":"HTTP 404","code":"HTTP_ERROR"} instead of code: undefined.

All error handling is now unified across the project. Ready for re-review!

@jwfing jwfing merged commit 2f8b41f into InsForge:main Mar 17, 2026
2 checks passed
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.

[1 point][Bug] unified error handler

2 participants