Skip to content

Add DeadlineExceeded error handling and improve error mapping#10

Merged
w1am merged 1 commit intomasterfrom
add-deadline-exceeded-error-handling
Apr 13, 2026
Merged

Add DeadlineExceeded error handling and improve error mapping#10
w1am merged 1 commit intomasterfrom
add-deadline-exceeded-error-handling

Conversation

@w1am
Copy link
Copy Markdown
Contributor

@w1am w1am commented Apr 13, 2026

Summary

  • Add DeadlineExceededError variant to ErrorKind enum and map kurrentdb::Error::DeadlineExceeded to it
  • Map kurrentdb::Error::ServerError and kurrentdb::Error::ConnectionClosed to UnavailableError for better resilience handling

Test plan

  • Verify bridge client correctly surfaces deadline exceeded errors to Node.js consumers
  • Verify server errors and connection closed errors are reported as unavailable
  • Confirm existing error mappings are unaffected

Map additional kurrentdb error variants to appropriate ErrorKind values:
- DeadlineExceeded -> DeadlineExceededError
- ServerError -> UnavailableError
- ConnectionClosed -> UnavailableError
@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Add deadline exceeded error handling and improve error mapping

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add DeadlineExceededError variant to ErrorKind enum
• Map kurrentdb::Error::DeadlineExceeded to new error kind
• Map ServerError and ConnectionClosed to UnavailableError
• Update error serialization to handle deadline exceeded errors
Diagram
flowchart LR
  A["kurrentdb::Error variants"] -->|"DeadlineExceeded"| B["DeadlineExceededError"]
  A -->|"ServerError, ConnectionClosed"| C["UnavailableError"]
  B --> D["Error serialization"]
  C --> D
Loading

Grey Divider

File Changes

1. crates/bridge/src/error.rs ✨ Enhancement +6/-1

Enhance error handling with deadline exceeded support

• Added DeadlineExceededError variant to ErrorKind enum
• Extended error mapping to handle ServerError and ConnectionClosed as UnavailableError
• Added DeadlineExceeded error mapping to new DeadlineExceededError variant
• Updated error serialization match statement to handle DeadlineExceededError case

crates/bridge/src/error.rs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Apr 13, 2026

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)   🖥 UI issues (0)   🎨 UX Issues (0)
🐞\ ◔ Observability (1)

Grey Divider


Remediation recommended

1. Unavailable drops error details 🐞
Description
kurrentdb::Error::ServerError(_) and ConnectionClosed are mapped to ErrorKind::UnavailableError
while discarding the original error payload/message, and create_js_error renders UnavailableError as
just the enum Debug string with no metadata detail. This makes Node.js consumers receive an error
with name/message "UnavailableError" but without the underlying cause, significantly reducing
debuggability for server-side failures.
Code

crates/bridge/src/error.rs[R21-22]

+            kurrentdb::Error::ServerError(_) => ErrorKind::UnavailableError,
+            kurrentdb::Error::ConnectionClosed => ErrorKind::UnavailableError,
Evidence
The new mapping explicitly discards the ServerError payload and converts both ServerError and
ConnectionClosed into UnavailableError. When emitting the JS error, UnavailableError’s message is
format!("{:?}", kind) and the metadata "detail" is only attached for UnknownError, so the original
context cannot reach JS consumers for these now-remapped errors.

crates/bridge/src/error.rs[17-30]
crates/bridge/src/error.rs[60-90]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ServerError(_)` and `ConnectionClosed` are mapped to `ErrorKind::UnavailableError` but the underlying message is discarded; the JS error generated for `UnavailableError` contains no detail metadata.

## Issue Context
This PR intentionally remaps `kurrentdb::Error::ServerError(_)` and `kurrentdb::Error::ConnectionClosed` to `UnavailableError` for resilience handling, but we still need to preserve the original message for debugging.

## Fix Focus Areas
- crates/bridge/src/error.rs[6-15]
- crates/bridge/src/error.rs[17-30]
- crates/bridge/src/error.rs[53-94]

## Suggested fix approach
- Change `ErrorKind::UnavailableError` to carry detail, e.g. `UnavailableError(String)` or `UnavailableError { detail: String }`.
- In `From<kurrentdb::Error>`, map:
 - `GrpcConnectionError(e)` / `ServerError(e)` / `ConnectionClosed` to `UnavailableError(err.to_string())` (or an appropriate detail string).
- In `create_js_error`, use that detail for `error_message` and/or attach it to `metadata.detail` similarly to `UnknownError`, while keeping `name` as `"UnavailableError"` so retry logic still works.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@w1am w1am merged commit 65f2777 into master Apr 13, 2026
2 checks passed
@w1am w1am deleted the add-deadline-exceeded-error-handling branch April 13, 2026 09:49
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 participant