Skip to content

fix: package comments, error return and deadlock fix#142

Merged
rustatian merged 2 commits intomasterfrom
fix/return-correct-error
Feb 1, 2026
Merged

fix: package comments, error return and deadlock fix#142
rustatian merged 2 commits intomasterfrom
fix/return-correct-error

Conversation

@rustatian
Copy link
Member

@rustatian rustatian commented Feb 1, 2026

Description of Changes

  • Fix deadlock in the status logic.
  • Correctly return error from the pool's channel.
  • Add package comments.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

  • All commits in this PR are signed (git commit -s).
  • The reason for this PR is clearly provided (issue no. or explanation).
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • Any user-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.

Summary by CodeRabbit

  • New Features

    • Improved PHP worker error propagation for better diagnostics.
  • Bug Fixes

    • Fixed potential null pointer issue in error handling.
    • Enhanced file error detection during parsing operations.
    • Improved mutex synchronization reliability.
  • Documentation

    • Added package-level documentation comments for better code clarity.
  • Chores

    • Updated CI environment to PHP 8.5.
    • Updated artifact download action to latest version.
    • Minor code formatting improvements.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
@rustatian rustatian self-assigned this Feb 1, 2026
Copilot AI review requested due to automatic review settings February 1, 2026 08:16
@rustatian rustatian added the bug Something isn't working label Feb 1, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 1, 2026

📝 Walkthrough

Walkthrough

This PR adds package-level documentation comments across multiple files, improves error handling in file operations and worker communication, refactors mutex management using defer patterns, and updates GitHub Actions workflows with PHP version 8.5 and artifact download action v5.

Changes

Cohort / File(s) Summary
Documentation Comments
api/interfaces.go, codec/codec.go, plugin.go, protoc_plugins/protoc-gen-php-grpc/main.go, protoc_plugins/protoc-gen-php-grpc/php/generate.go
Package and file-level documentation comments added to describe gRPC plugin components, codec behavior, and code generation functionality.
Error Handling Improvements
parser/parse.go, proxy/proxy.go
File open errors now properly checked and returned; worker error propagation changed to surface actual worker errors instead of local errors.
Mutex & Status Handling
health_server.go, server.go
SetServingStatus uses defer for mutex unlock; interceptor error/status handling refactored to use condition-based switch with nil-safe error handling and explicit default case.
CI Workflow & Formatting
.github/workflows/linux.yml, protoc_plugins/protoc-gen-php-grpc/php/template.go
PHP version bumped to 8.5; artifact download action updated to v5; consolidation of test script into single inline string; whitespace cleanup in template function map.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A hop, skip, and bounds of care,
Package docs float through the air,
Errors caught with proper grace,
Defers unlock their mutex embrace,
Clean and clear, this code's now fair! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately summarizes the three main changes: package comments, error return fix, and deadlock fix. It is concise, specific, and clearly communicates the primary objectives.
Description check ✅ Passed The PR description includes the required sections with specific change details, license acceptance, and a completed checklist. However, it lacks a 'Reason for This PR' section with an issue reference.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/return-correct-error

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.

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Copy link

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

This PR addresses three critical bug fixes and adds missing package documentation:

Changes:

  • Fixed a deadlock in the health check server by properly positioning the mutex unlock using defer
  • Corrected error return in the proxy to return the payload error instead of the wrong error variable
  • Added package-level comments to all packages following Go documentation conventions

Reviewed changes

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

Show a summary per file
File Description
health_server.go Fixed deadlock by moving defer h.mu.Unlock() to immediately after lock acquisition in SetServingStatus
proxy/proxy.go Fixed bug where wrong error variable was returned; now correctly returns pl.Error()
server.go Enhanced status code determination logic to handle nil error case with explicit default
parser/parse.go Added proper error handling for file open operation
plugin.go Added package comment describing the gRPC server plugin
api/interfaces.go Added package comment for API interfaces
codec/codec.go Added package comment for codec implementation
protoc_plugins/protoc-gen-php-grpc/main.go Added command documentation comment
protoc_plugins/protoc-gen-php-grpc/php/generate.go Added package comment for PHP code generation
protoc_plugins/protoc-gen-php-grpc/php/template.go Removed extraneous blank lines for cleaner formatting
go.work.sum Automatic dependency checksum updates

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

@rustatian rustatian merged commit 7b5085f into master Feb 1, 2026
7 checks passed
@rustatian rustatian deleted the fix/return-correct-error branch February 1, 2026 08:22
@codecov
Copy link

codecov bot commented Feb 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (a930720) to head (780ec78).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@      Coverage Diff      @@
##   master   #142   +/-   ##
=============================
=============================

☔ 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.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants