Skip to content

Fix error handling issues in environment group services #807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 30, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 4, 2025

This PR addresses multiple error handling issues found in the Power Platform Terraform provider's environment group services, improving error context, debugging capabilities, and overall reliability.

Issues Fixed

1. Missing Error Wrapping with Context

Files: internal/services/environment_groups/api_environment_group.go

Added proper error wrapping in all API methods to provide better context for debugging:

// Before
if err != nil {
    return nil, err
}

// After  
if err != nil {
    return nil, fmt.Errorf("failed to create environment group: %w", err)
}

2. Error Value Usage After Control Flow

Files: internal/services/environment_group_rule_set/api_environment_group_rule_set.go

Fixed incorrect error wrapping when handling http.StatusNoContent responses:

// Before - wrapping potentially nil error
if resp.HttpResponse.StatusCode == http.StatusNoContent {
    return nil, customerrors.WrapIntoProviderError(err, customerrors.ERROR_OBJECT_NOT_FOUND, "rule set '%s' not found")
}

// After - explicitly creating informative error
if resp.HttpResponse.StatusCode == http.StatusNoContent {
    return nil, customerrors.WrapIntoProviderError(
        fmt.Errorf("rule set '%s' not found", environmentGroupId),
        customerrors.ERROR_OBJECT_NOT_FOUND,
        fmt.Sprintf("rule set '%s' not found", environmentGroupId),
    )
}

3. Silent Failure in Delete Method

Files: internal/services/environment_group_rule_set/api_environment_group_rule_set.go

Improved error handling in the delete method to ensure proper error context:

// Before
_, err = client.Api.Execute(...)
return err

// After
_, err = client.Api.Execute(...)
if err != nil {
    return fmt.Errorf("failed to delete environment group rule set: %w", err)
}
return nil

4. Improved Parameter Length Check Error Message

Files: internal/services/environment_group_rule_set/api_environment_group_rule_set.go

Enhanced error message clarity and added explanatory comment:

// If empty Parameters is a valid API response, remove the following check.
// Otherwise, consider clarifying its necessity and error messaging.
if len(environmentGroupRuleSet.Parameters) == 0 {
    return nil, fmt.Errorf("no environment group ruleset parameters found for environment group id %s", environmentGroupId)
}

5. Incorrect Error Message in Configure Method

Files: internal/services/environment_group_rule_set/resource_environment_group_rule_set.go

Fixed misleading error message to reflect the actual expected type:

// Before
fmt.Sprintf("Expected *http.Client, got: %T. Please report this issue to the provider developers.", req.ProviderData)

// After
fmt.Sprintf("Expected *api.ProviderClient, got: %T. Please report this issue to the provider developers.", req.ProviderData)

6. Pointer Model Usage in State Management

Files: internal/services/environment_group_rule_set/resource_environment_group_rule_set.go

Fixed incorrect pointer dereferencing in the Create method:

// Before
resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)

// After
resp.Diagnostics.Append(resp.State.Set(ctx, *plan)...)

Testing

  • All existing unit tests continue to pass
  • Linting passes with no issues
  • Build succeeds without errors
  • Code coverage remains at 73.4% for the affected services

Impact

These changes improve:

  • Debugging experience: Better error context makes it easier to trace issues
  • Reliability: Proper error handling prevents silent failures
  • Maintainability: Consistent error patterns across the codebase
  • User experience: More informative error messages for troubleshooting

Fixes #806.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: mawasile <50197777+mawasile@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] [Copilot] Error Handling Issues Fix error handling issues in environment group services Jun 4, 2025
@Copilot Copilot AI requested a review from mawasile June 4, 2025 07:16
Copilot finished work on behalf of mawasile June 4, 2025 07:17
@mawasile
Copy link
Contributor

mawasile commented Jun 4, 2025

@copilot please run the precommit tools and run changie to report change log

…vices

Co-authored-by: mawasile <50197777+mawasile@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jun 4, 2025

@copilot please run the precommit tools and run changie to report change log

Completed both requested tasks:

Precommit tools: Ran linting (0 issues), unit tests (environment group services pass), and build (successful)

Changie changelog: Created changelog entry for error handling fixes referencing issue #806

See commit fb53e9d for the changelog entry.

Copilot finished work on behalf of mawasile June 4, 2025 12:07
@mawasile mawasile marked this pull request as ready for review June 4, 2025 12:14
@Copilot Copilot AI review requested due to automatic review settings June 4, 2025 12:14
@mawasile mawasile requested a review from a team as a code owner June 4, 2025 12:14
@mawasile mawasile added ai assisted Pull requests that have be fully or partially implemented using Copilot or AI copilot fixed using GitHub copilot autonomous agent labels Jun 4, 2025
Copy link
Contributor

@Copilot 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 improves error handling in the Power Platform Terraform provider's environment group services by adding additional context to errors and ensuring consistent error reporting.

  • Enhanced error wrapping with fmt.Errorf in environment group create, update, get, and delete methods.
  • Fixed error handling when dealing with HTTP no-content responses and improper pointer dereferencing in resource state functions.
  • Updated error messages in the rule set configure and API methods for clarity.

Reviewed Changes

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

File Description
internal/services/environment_groups/api_environment_group.go Added contextual error wrapping in create, update, get, and remove methods.
internal/services/environment_group_rule_set/resource_environment_group_rule_set.go Corrected error message and pointer usage in state set, and updated configure error messaging.
internal/services/environment_group_rule_set/api_environment_group_rule_set.go Improved error wrapping for tenant retrieval, GET, create, update, and delete methods with clearer messages.
.changes/unreleased/fix_error_handling_environment_groups.yaml Updated changelog file to reflect improved error handling.

@mawasile mawasile added the ai found Issues and Bugs that were found using AI label Jun 4, 2025
mattdot
mattdot previously approved these changes Jun 4, 2025
eduardodfmex
eduardodfmex previously approved these changes Jun 17, 2025
@mawasile mawasile dismissed stale reviews from eduardodfmex and mattdot via ac30750 June 17, 2025 15:34
@mawasile mawasile merged commit 4acc877 into main Jun 30, 2025
11 checks passed
@mawasile mawasile deleted the copilot/fix-806 branch June 30, 2025 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ai assisted Pull requests that have be fully or partially implemented using Copilot or AI ai found Issues and Bugs that were found using AI copilot fixed using GitHub copilot autonomous agent
Projects
None yet
Development

Error loading sessions

Retrying...

Successfully merging this pull request may close these issues.

[Copilot] Error Handling Issues
4 participants