replace errgroup with conc pool in WriteExecute method

[AltuisticIsopod]

Replace errgroup with conc pool in WriteExecute method

Description

• Replace errgroup with conc pool for parallel write/delete operations
• Use errors.As() instead of type assertion for authentication error detection
• Handle wrapped errors from conc pool using errors.As() in both client and test
• Add nil check before dereferencing singleResponse to prevent panics
• Update test to use errors.As() for proper error unwrapping

This PR only moved "Write" to use Conc

References

openfga/go-sdk#193

Review Checklist

• I have clicked on "allow edits by maintainers".
• I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
• The correct base branch is being used, if not main
• I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

• Refactor
  • Enhanced concurrent request processing with improved error handling and resource management for better reliability and performance.

[coderabbitai]

Walkthrough

Replaces per-chunk parallel execution from an errgroup-based approach to a pool-based concurrency model in the Go client template. Introduces controlled worker pools with context propagation, refactors authentication error handling via errors.As, and updates WriteExecute and DeleteExecute methods to iterate over chunks through pool tasks.

Changes

Cohort / File(s)

Summary

Go Client Concurrency Refactoring config/clients/go/template/client/client.mustache

Replaces errgroup-based per-chunk parallelism with worker pools (writePool/deletePool); adds errors package import for error inspection; converts authentication error handling from type assertion to errors.As; refactors WriteExecute and DeleteExecute to collect per-chunk responses via pool tasks instead of explicit index-based errgroup capturing; shifts result aggregation from errgroup.Wait() to pool.Wait()

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant OldErrGroup as Old: ErrGroup
    participant NewPool as New: Worker Pool
    participant Chunk

    Client->>OldErrGroup: WriteExecute(chunks)
    loop Per Chunk (indexed)
        OldErrGroup->>Chunk: Go(i) → execute chunk[i]
        Chunk-->>OldErrGroup: response
    end
    OldErrGroup->>OldErrGroup: Wait()
    OldErrGroup-->>Client: combined result + error

    Client->>NewPool: WriteExecute(chunks)
    rect rgb(200, 220, 250)
        Note over NewPool: Initialize pool with<br/>max goroutines
    end
    loop Per Chunk (no index capture)
        NewPool->>Chunk: Go(task) → execute chunk
        Chunk-->>NewPool: response (collected)
    end
    NewPool->>NewPool: Wait()
    NewPool-->>Client: combined result + error
    
    rect rgb(240, 200, 200)
        Note over NewPool: Auth errors via errors.As<br/>instead of type assertion
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Concurrency pattern shift: Review the pool initialization, goroutine limit configuration, and correctness of chunk iteration and response collection logic
• Error handling refactoring: Verify errors.As correctly replaces the previous type assertion for authentication errors and maintains existing error semantics
• Response aggregation: Ensure per-chunk responses are properly accumulated and the final combined result construction remains functionally equivalent to the errgroup approach
• Context propagation: Confirm context is correctly threaded through pool tasks and cleanup behavior is preserved

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "replace errgroup with conc pool in WriteExecute method" accurately describes the primary refactoring in the changeset. The title is specific and clear, correctly identifying the main change as a replacement of the concurrency mechanism from errgroup to a pool-based approach in the WriteExecute method. While the changes also include error handling improvements (switching to errors.As) and nil checks, the core objective and most significant change is the concurrency model replacement, which is properly captured in the title. The title is neither vague nor misleading, and it provides sufficient clarity for a developer scanning the repository history to understand the nature of this change.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

config/clients/go/template/client/client.mustache (2)

1786-1814: Add defensive nil check before dereferencing response.

The writePool implementation looks correct with proper context propagation and error handling. However, when aggregating responses at lines 1856-1857, there's a potential nil pointer dereference if writeResponse is nil.

While WriteExecute appears to always return a non-nil response even on error, adding a defensive nil check would make the code more robust against future changes.

Consider adding a nil check in the aggregation loop:

 for _, writeResponse := range writeResponses {
+    if writeResponse != nil {
         response.Writes = append(response.Writes, writeResponse.Writes...)
+    }
 }

---

1825-1862: Add defensive nil checks in response aggregation.

The deletePool implementation correctly mirrors the writePool pattern. However, the response aggregation at lines 1856-1862 should include nil checks to prevent potential panics.

Apply this diff to add defensive nil checks:

 for _, writeResponse := range writeResponses {
+    if writeResponse != nil {
         response.Writes = append(response.Writes, writeResponse.Writes...)
+    }
 }

 for _, deleteResponse := range deleteResponses {
+    if deleteResponse != nil {
         response.Deletes = append(response.Deletes, deleteResponse.Deletes...)
+    }
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d15c31e and 055b203.

📒 Files selected for processing (1)

• config/clients/go/template/client/client.mustache (5 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

config/**/*.mustache

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Validate mustache syntax and variable references across all template files, including CHANGELOG.md.mustache

Files:

• config/clients/go/template/client/client.mustache

config/**/*.{json,mustache}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Never hardcode API keys or credentials in configuration or template files

Files:

• config/clients/go/template/client/client.mustache

🔇 Additional comments (2)

config/clients/go/template/client/client.mustache (2)

7-7: LGTM! Standard library import for error unwrapping.

The errors package is correctly imported for the errors.As() calls used in authentication error handling below.

---

1803-1808: LGTM! Correct use of errors.As for authentication error detection.

The switch from type assertion to errors.As() properly handles wrapped errors from the conc pool. Authentication errors fail fast (line 1805), while other errors are captured in the response structure for per-tuple error tracking.

[dyeam0]

Hi @AltuisticIsopod, we'll assign someone to review your PR. Thanks.