Skip to content

fix(jsonrpc): prevent nil pointer panic on unregistered service path#3314

Open
cvictory wants to merge 1 commit into
developfrom
task/fix-jsonrpc-nil-panic-3310-20260429-192213
Open

fix(jsonrpc): prevent nil pointer panic on unregistered service path#3314
cvictory wants to merge 1 commit into
developfrom
task/fix-jsonrpc-nil-panic-3310-20260429-192213

Conversation

@cvictory
Copy link
Copy Markdown
Contributor

Bug

Fix

  • Check the ok return value from ExporterMap().Load(path)
  • When !ok, return perrors.Errorf("service not found: %s", path) instead of proceeding to type-assert a nil value

Verification

  • New test: TestHandlePkg_UnregisteredPath — sends valid JSON-RPC request to unregistered path, asserts HTTP 500 with "service not found" (no panic)
  • Existing tests: All 6 TestHandlePkg_ContentType sub-tests pass
  • Full package: All 9 tests in protocol/jsonrpc pass
  • Static analysis: go vet ./protocol/jsonrpc/ passes

Closes #3310

…3310)

Check the ok return value from ExporterMap().Load(path) in serveRequest().
When the path is not registered, return a descriptive "service not found"
error instead of panicking on a nil type assertion.

Closes #3310
@sonarqubecloud
Copy link
Copy Markdown

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.22%. Comparing base (ea93b8a) to head (6499e1b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3314      +/-   ##
==========================================
+ Coverage   50.79%   51.22%   +0.43%     
==========================================
  Files         488      488              
  Lines       44203    36566    -7637     
==========================================
- Hits        22451    18730    -3721     
+ Misses      20242    16322    -3920     
- Partials     1510     1514       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AlexStocks AlexStocks requested a review from Copilot April 29, 2026 13:00
@AlexStocks AlexStocks changed the base branch from main to develop April 29, 2026 13:01
Copy link
Copy Markdown
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

Fixes a JSON-RPC server panic when requests target an unregistered service path by validating exporter lookup results and returning a clear error instead of type-asserting a nil value.

Changes:

  • Add ok check around ExporterMap().Load(path) in serveRequest() and return service not found when missing.
  • Add a regression test to ensure unregistered paths return HTTP 500 with an explanatory error (and do not panic).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
protocol/jsonrpc/server.go Prevents nil pointer panic by handling missing exporter lookups with a returned error.
protocol/jsonrpc/server_test.go Adds coverage for unregistered-path requests to verify error response and no panic.

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

@AlexStocks
Copy link
Copy Markdown
Contributor

@cvictory 展图,todo:
1 把文件冲突解决下。
2 后面提pr,先提到 develop 分支。
thx again。

@Alanxtl Alanxtl added the 3.3.2 version 3.3.2 label Apr 30, 2026
@Alanxtl
Copy link
Copy Markdown
Contributor

Alanxtl commented May 15, 2026

this issue have been solved in #3312 is this pr still needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.3.2 version 3.3.2 ☢️ Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Nil pointer dereference panic when JSON-RPC request targets unregistered service path

5 participants