Skip to content

fix(server): map ERROR_CODE_CANCELLED to gRPC Canceled not Internal#2978

Open
ManthanNimodiya wants to merge 1 commit into
Permify:masterfrom
ManthanNimodiya:fix/grpc-status-cancelled-mapping
Open

fix(server): map ERROR_CODE_CANCELLED to gRPC Canceled not Internal#2978
ManthanNimodiya wants to merge 1 commit into
Permify:masterfrom
ManthanNimodiya:fix/grpc-status-cancelled-mapping

Conversation

@ManthanNimodiya
Copy link
Copy Markdown

@ManthanNimodiya ManthanNimodiya commented May 27, 2026

Problem

GetStatus maps all 5xxx error codes to codes.Internal via a range check.

This incorrectly includes ERROR_CODE_CANCELLED (5001), so every context cancellation is logged as ERROR-level ,rpc error: code = Internal desc = ERROR_CODE_CANCELLED, by gRPC interceptors, flooding production logs in distributed mode.

Fix

Added explicit cases before the range fallthrough:

  • ERROR_CODE_CANCELLED → codes.Canceled
  • ERROR_CODE_NOT_IMPLEMENTED → codes.Unimplemented
  • ERROR_CODE_SERIALIZATION → codes.Aborted

Added error_test.go with 9 test cases.

Closes #2700

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced error response handling to correctly map specific error conditions to appropriate gRPC status codes, improving error clarity for API consumers.
  • Tests

    • Added comprehensive unit tests to validate error code mappings and ensure consistent error handling behavior across various scenarios.

Review Change Stack

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 14f2d11b-d525-4cdf-8f2f-1d4e4856bbd4

📥 Commits

Reviewing files that changed from the base of the PR and between 70b1110 and 519c54d.

📒 Files selected for processing (2)
  • internal/servers/error.go
  • internal/servers/error_test.go

📝 Walkthrough

Walkthrough

The PR adds explicit error-code-to-gRPC-status mapping in the GetStatus function to correctly remap certain base error codes before falling back to the existing numeric range-based mapping. It introduces a comprehensive table-driven test to validate all mapping paths.

Changes

Error code to gRPC status mapping

Layer / File(s) Summary
Error code to gRPC status mapping
internal/servers/error.go, internal/servers/error_test.go
GetStatus adds a switch that maps base.ErrorCode_CANCELLEDcodes.Canceled, base.ErrorCode_NOT_IMPLEMENTEDcodes.Unimplemented, and base.ErrorCode_SERIALIZATIONcodes.Aborted before falling back to range-based mapping. TestGetStatus is a table-driven test covering explicit overrides, range-based mappings for authentication/validation/not-found/internal errors, unknown error defaults to codes.Internal, and pass-through of pre-existing status.Error values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A hop, a switch, a fix so fine,
ERROR_CODE_CANCELLED now maps by design,
From Internal's shadow to Canceled's embrace,
The gRPC codes find their rightful place! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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 identifies the main change: mapping ERROR_CODE_CANCELLED from Internal to gRPC Canceled status code.
Linked Issues check ✅ Passed The PR fully addresses issue #2700 by explicitly mapping ERROR_CODE_CANCELLED (and related 5xxx codes) to appropriate gRPC status codes to prevent noisy Internal error logs.
Out of Scope Changes check ✅ Passed All changes are within scope: explicit gRPC status mapping for ERROR_CODE_CANCELLED and two related codes, plus comprehensive test coverage for the mapping logic.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@ManthanNimodiya
Copy link
Copy Markdown
Author

@EgeAytin @ucatbas @tolgaozen, have a look when you get chance and lmk for any required changes

@ManthanNimodiya
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request May 28, 2026
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.

ERROR_CODE_CANCELLED, "context canceled" errors in logs after enabling the distributed mode on

1 participant