go-sdk icon indicating copy to clipboard operation
go-sdk copied to clipboard

[BUG]: GET request for repos/OWNER/REPO/properties/values broken

Open felixlut opened this issue 1 year ago • 4 comments

What happened?

I'm playing around with this sdk as a means of contributing to https://github.com/integrations/terraform-provider-github/issues/1956, and as such my focus is on the endpoints related to repository custom properties.

The issue I'm facing occurs when calling octokitClient.Repos().ByOwnerId(owner).ByRepoId(repoName).Properties().Values().Get(), which is mapped to the repos/OWNER/REPO/properties/values GET endpoint. I'll list the code I'm using further down in the message.

When running the code on a repository where no custom properties are set, it works as intended by returning an empty list. But when using it on a repo where any custom property is set, the error below is the result. I've played around with properties of all available types (true_false, single_select, multi_select, and text), and all of them have this error. Error:

panic: index is empty.

For example, if I add a custom property called text with a value of test, I get the following response while using the gh cli (gh api repos/YesWeKanske/test-custom-properties/properties/values):

[
  {
    "property_name": "text",
    "value": "test"
  }
]

But I get the index is empty error with the code I've posted below.

I've ran a debugger and tracked this error down to this line in the microsoft/kiota-serialization-json-go library, which is being called from this line in pkg/github/models/custom_property_value.go. From what I can gather this part of the code is for converting the value (in this case the string "test") of a custom property to the structs the Kiota framework can parse (or something). Point is that it deals with the value of a custom property.

I'd be lying if I said I understand exactly what Kiota is doing under the hood, but the parseNode.GetChildNode("") looks weird to me. Calling that with an empty string will always render in an error, and there are quite a few examples throughout the codebase of this occurring. I've only tested it with the repository custom properties, but I'd assume the same errors would occur for any of the other resources as well.

My code:

package main

import (
	"context"
	"fmt"
	"log"
	"os"
	"time"

	abs "github.com/microsoft/kiota-abstractions-go"
	"github.com/octokit/go-sdk/pkg"
)

func main() {
	octokitClient, err := pkg.NewApiClient(
		pkg.WithUserAgent("my-user-agent"),
		pkg.WithRequestTimeout(5*time.Second),
		pkg.WithBaseUrl("https://api.github.com"),
		pkg.WithTokenAuthentication(os.Getenv("GITHUB_TOKEN")),
	)
	ctx := context.Background()

	if err != nil {
		log.Fatalf("error creating client: %v", err)
	}

	owner := "YesWeKanske"
	repoName := "test-custom-properties"

	defaultRequestConfig := &abs.RequestConfiguration[abs.DefaultQueryParameters]{
		QueryParameters: &abs.DefaultQueryParameters{},
	}
	repoURL := octokitClient.Repos().ByOwnerId(owner).ByRepoId(repoName)
	propURL := repoURL.Properties().Values()

	props, err := propURL.Get(ctx, defaultRequestConfig)
	if err != nil {
		panic(err)
	}
	fmt.Println(props)
}

Versions

I've used the latest version of the provider at the time of writing this bug report (go get github.com/octokit/go-sdk@a74a3a2a13654bd84794fd91a6a0fbc96f883722, where a74a3a2a13654bd84794fd91a6a0fbc96f883722 is a commit to this repo). My go.mod file looks like so:

module example/hello

go 1.22.3

require (
	github.com/microsoft/kiota-abstractions-go v1.6.0
	github.com/octokit/go-sdk v0.0.21-0.20240709170617-a74a3a2a1365
)

require (
	github.com/cjlapao/common-go v0.0.39 // indirect
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/go-logr/logr v1.4.1 // indirect
	github.com/go-logr/stdr v1.2.2 // indirect
	github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
	github.com/google/uuid v1.6.0 // indirect
	github.com/kfcampbell/ghinstallation v0.0.6 // indirect
	github.com/microsoft/kiota-http-go v1.3.3 // indirect
	github.com/microsoft/kiota-serialization-form-go v1.0.0 // indirect
	github.com/microsoft/kiota-serialization-json-go v1.0.7 // indirect
	github.com/microsoft/kiota-serialization-multipart-go v1.0.0 // indirect
	github.com/microsoft/kiota-serialization-text-go v1.0.0 // indirect
	github.com/pmezard/go-difflib v1.0.0 // indirect
	github.com/std-uritemplate/std-uritemplate/go v0.0.55 // indirect
	github.com/stretchr/testify v1.9.0 // indirect
	go.opentelemetry.io/otel v1.24.0 // indirect
	go.opentelemetry.io/otel/metric v1.24.0 // indirect
	go.opentelemetry.io/otel/trace v1.24.0 // indirect
	gopkg.in/yaml.v3 v3.0.1 // indirect
)
```

### Code of Conduct

- [X] I agree to follow this project's Code of Conduct

felixlut avatar Jul 14 '24 16:07 felixlut

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

github-actions[bot] avatar Jul 14 '24 16:07 github-actions[bot]

I'll note that a workaround to this is fetching the repository directly via octokitClient.Repos().ByOwnerId(owner).ByRepoId(repoName).Get(), which parses the custom properties of the repo correctly. Looks something like:

repo, err := octokitClient.Repos().ByOwnerId(owner).ByRepoId(repoName).Get(...)
repoProps := repo.GetCustomProperties().GetAdditionalData()

felixlut avatar Jul 14 '24 17:07 felixlut

Hit the same trying to get the current user:

error in user_request_builder

package dev

import (
	"context"
	"fmt"
	"os"

	octokit "github.com/octokit/go-sdk/pkg"
)

func main() {
	gh, err := octokit.NewApiClient(
		octokit.WithUserAgent("my-thing"),
		octokit.WithTokenAuthentication(os.Getenv("GITHUB_TOKEN")),
	)
	if err != nil {
		fmt.Println("error: ", err)
	}

	ctx := context.Background()

	user, err := gh.User().Get(ctx, nil)
	if err != nil {
		fmt.Println("error: ", err) // panic: index is empty
	}

	fmt.Println("user: ", user)
}

colbylwilliams avatar Aug 21 '24 16:08 colbylwilliams

Bumping into the same issue with client.Repos().ByOwnerId(owner).ByRepoId(repo).Rulesets().ByRuleset_id(rulesetId).Get(), which is mapped to /repos/{owner}/{repo}/rulesets/{ruleset_id}. The parseNode.GetChildNode("") causes an error of index is empty at this line as well.

// err == index is empty
response, err := client.Repos().ByOwnerId(owner).ByRepoId(repo).Rulesets().ByRuleset_id(rulesetId).Get(context.background(), nil) 

HowHow06 avatar Oct 01 '24 04:10 HowHow06