@diofeher Due to the length of time it has taken to get this in, I'll step in and complete the PR description. After running some local tests this is what I came up with:

If an error occurs that is NOT a github.ErrorResponse or is a github.ErrorResponse with a status code other than 304 or 404 the current implementation will:

Not return from the function and continue executing and try to access resp.Header and ruleset.Name. Since both resp and ruleset are nil when there's an error we get a SIGSEGV

@nickfloyd Would there be any value in having some kind of regression test for that scenario? Maybe it could be used in the future to ensure other resources don't end up in a similar situation?

@nickfloyd Would there be any value in having some kind of regression test for that scenario? Maybe it could be used in the future to ensure other resources don't end up in a similar situation?

I'm always up for more tests 😉. I think the main challenge here is that, currently our testing suite does not handle integration or e2e scenarios very well. So when testing we have to use a real org and in this case one that has org rulesets.

I know @stevehipwell has been working to make that whole experience better as well.

Verified with:

terraform import github_organization_ruleset.test 999999999

terraform {
  required_providers {
    github = {
      source  = "integrations/github"
      version = "99.0.0" 
    }
  }
}

provider "github" {
  owner = "integrations"
}

# Using a non-existent ID 999999999
resource "github_organization_ruleset" "test" {
  name        = "test-ruleset"
  target      = "branch"
  enforcement = "disabled"

  conditions {
    ref_name {
      include = ["~ALL"]
      exclude = []
    }
  }

  rules {
    creation = true
  }
}

output "test" {
  value = "Testing PR #2705 fix"
}

Returns a 404 and not a segfault now.

@nickfloyd this is an instance of the defective error pattern I mentioned on an issue/PR the other week. I think we need to get a general issue open to track resolving the other places where this pattern is present.

@nickfloyd this is an instance of the defective error pattern I mentioned on an issue/PR the other week. I think we need to get a general issue open to track resolving the other places where this pattern is present.

Agreed. I'll try to put an issue together shortly, unless you get to it before I do.

@nickfloyd I've created #2957.