Skip to content
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

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

Open
felixlut opened this issue Jul 14, 2024 · 4 comments
Open

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

felixlut opened this issue Jul 14, 2024 · 4 comments
Labels
Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented

Comments

@felixlut
Copy link

felixlut commented Jul 14, 2024

What happened?

I'm playing around with this sdk as a means of contributing to integrations/terraform-provider-github#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 felixlut added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Jul 14, 2024
Copy link

👋 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! 🚀

@felixlut
Copy link
Author

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 felixlut changed the title [BUG]: Get request for repos/OWNER/REPO/properties/values broken [BUG]: GET request for repos/OWNER/REPO/properties/values broken Jul 15, 2024
@nickfloyd nickfloyd moved this from 🆕 Triage to 🛑 Blocked/Awaiting Response in 🧰 Octokit Active Jul 22, 2024
@colbylwilliams
Copy link

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)
}

@HowHow06
Copy link

HowHow06 commented Oct 1, 2024

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) 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented
Projects
Status: 🛑 Blocked/Awaiting Response
Development

No branches or pull requests

3 participants