Skip to content

Fix JSON-RPC request error handling and pending request cleanup#219

Open
gioalex07 wants to merge 2 commits intogogcom:masterfrom
gioalex07:codex/fix-jsonrpc-error-handling
Open

Fix JSON-RPC request error handling and pending request cleanup#219
gioalex07 wants to merge 2 commits intogogcom:masterfrom
gioalex07:codex/fix-jsonrpc-error-handling

Conversation

@gioalex07
Copy link
Copy Markdown

Summary

This PR fixes a few issues in the JSON-RPC layer that could affect plugin stability, especially around internal
requests like refresh_credentials().

What Changed

  • Return immediately after sending InvalidParams for requests with bad arguments
  • Stop trying to respond to notifications with invalid params
  • Clean up pending _requests_futures entries after handling a response
  • Abort and clear pending internal requests when the connection closes
  • Rebuild remote errors into their original specific types instead of always raising generic JsonRpcError
  • Add regression tests for:
    • invalid request params
    • invalid notification params
    • pending request cleanup after success/failure
    • aborted refresh_credentials() on close
    • preservation of specific remote error types

Root Cause

There were three related problems in jsonrpc.py:

  1. After detecting invalid params, the handler still continued execution and used bound_args even when binding had
    failed.
  2. Outgoing request futures were never removed from _requests_futures, even after receiving a response or closing
    the connection.
  3. Remote error responses were reconstructed as generic JsonRpcError, which discarded the original application-
    specific error type.

Impact

These issues could lead to:

  • unexpected UnboundLocalError after InvalidParams
  • hanging internal requests if the connection closed before a response arrived
  • memory/state leaks in _requests_futures
  • degraded error handling in plugins that rely on specific exceptions from refresh_credentials()

Validation

Validated with targeted local reproductions covering:

  • invalid request params no longer crashing the handler
  • invalid notifications no longer emitting bogus responses
  • remote errors being raised as their specific original types
  • pending request futures being cleared after response handling and on connection close

Notes

This PR is intentionally limited to the JSON-RPC layer and regression coverage around it.

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