Skip to content

fix(coprocessor): improve error handling in edge cases#2180

Merged
mergify[bot] merged 1 commit intomainfrom
antoniu/panics
Mar 26, 2026
Merged

fix(coprocessor): improve error handling in edge cases#2180
mergify[bot] merged 1 commit intomainfrom
antoniu/panics

Conversation

@antoniupop
Copy link
Copy Markdown
Collaborator

@antoniupop antoniupop commented Mar 26, 2026

Cleaner error handling.

@cla-bot cla-bot bot added the cla-signed label Mar 26, 2026
@antoniupop
Copy link
Copy Markdown
Collaborator Author

antoniupop commented Mar 26, 2026

@claude /pr-review

@antoniupop antoniupop marked this pull request as ready for review March 26, 2026 11:56
@antoniupop antoniupop requested a review from a team as a code owner March 26, 2026 11:56
@claude
Copy link
Copy Markdown

claude bot commented Mar 26, 2026 •
edited
Loading

Claude finished @antoniupop's request. — View run

@antoniupop antoniupop changed the title fix(copocessor): improve error handling in edge cases fix(coprocessor): improve error handling in edge cases Mar 26, 2026
claude[bot]
claude bot reviewed Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

[AUTOMATED]

Code Review

Good improvement! Converting panic!() and todo!() to proper Result error handling is the right approach for a coprocessor that should be resilient. All call sites are correctly updated.

I found a couple of issues worth addressing — one medium-severity diagnostic concern and one minor nit.

@claude
Copy link
Copy Markdown

claude bot commented Mar 26, 2026

The review was posted successfully. Here's a summary of what was done:

PR Review Summary for #2180

PR: fix(copocessor): improve error handling in edge cases
Verdict: COMMENT (non-blocking feedback)

What the PR Does

Converts panic!() and todo!() calls to proper Result<T, FhevmError> error handling in four functions:

  • trivial_encrypt_be_bytes — return type changed to Result, default match arm uses Err instead of panic!
  • generate_random_number — same pattern, panic! → Err(FhevmError::UnknownFheType(...))
  • check_fhe_operand_types — panic! → Err(FhevmError::UnsupportedFheTypes {...})
  • perform_fhe_operation_impl — panic! → Err(FhevmError::UnknownCastType {...}), todo!() → proper error

All call sites (4 total) were verified to be correctly updated.

Issues Found (2)

  1. Medium (tfhe_ops.rs:704): Empty input_types: vec![] in the error for check_fhe_operand_types loses diagnostic information that other error sites in the function provide.
  2. Nit (gpu_memory.rs:58): get_supported_ct_size_on_gpu still panics via .expect() — the panic is just moved, not removed. Fine if callers always pass valid types.

@antoniupop
Copy link
Copy Markdown
Collaborator Author

antoniupop commented Mar 26, 2026

@Mergifyio queue

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 26, 2026 •
edited
Loading

Merge Queue Status

  • 🟠 Waiting for queue conditions
  • ⏳ Enter queue
  • ⏳ Run checks
  • ⏳ Merge
Required conditions to enter a queue
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of [📌 queue -> configuration change requirements]:
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of [🔀 queue conditions]:
    • all of [📌 queue conditions of queue rule main]:
      • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • base = main
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=do-not-merge
      • any of [🛡 GitHub branch protection]:
        • check-success = common-pull-request/lint (bpr)
        • check-neutral = common-pull-request/lint (bpr)
        • check-skipped = common-pull-request/lint (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-success = coprocessor-cargo-test/cargo-tests (bpr)
        • check-neutral = coprocessor-cargo-test/cargo-tests (bpr)
        • check-skipped = coprocessor-cargo-test/cargo-tests (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-success = coprocessor-dependency-analysis/dependencies-check (bpr)
        • check-neutral = coprocessor-dependency-analysis/dependencies-check (bpr)
        • check-skipped = coprocessor-dependency-analysis/dependencies-check (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-skipped = gateway-contracts-deployment-tests/sc-deploy (bpr)
        • check-neutral = gateway-contracts-deployment-tests/sc-deploy (bpr)
        • check-success = gateway-contracts-deployment-tests/sc-deploy (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-skipped = kms-connector-tests/test-connector (bpr)
        • check-neutral = kms-connector-tests/test-connector (bpr)
        • check-success = kms-connector-tests/test-connector (bpr)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 26, 2026

Changed Lines Coverage

Coverage of added/modified lines: N/A

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 26, 2026 •
edited
Loading

Merge Queue Status

This pull request spent 2 hours 20 minutes 33 seconds in the queue, including 1 hour 40 minutes 17 seconds running CI.

Required conditions to merge

mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2180
93c1165
This was referenced Mar 26, 2026
Closed
Merged
mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2180
bdddbba
This was referenced Mar 26, 2026
Closed
Merged
Merged
mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2180
34cab75
@mergify mergify bot merged commit b174610 into main Mar 26, 2026
63 of 64 checks passed
@mergify mergify bot removed the merge-queued label Mar 26, 2026
@mergify mergify bot deleted the antoniu/panics branch March 26, 2026 16:26
@mergify mergify bot removed the queued label Mar 26, 2026
mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2180
5876e11
mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2180
9438595
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@claude claude[bot] claude[bot] left review comments

@PanGan21 PanGan21 PanGan21 approved these changes

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@antoniupop @PanGan21