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]: Repos[owner][repo].Issues.Comments[comment_id].GetAsync() always throws #117

Open
1 task done
IEvangelist opened this issue Aug 28, 2024 · 5 comments
Open
1 task done
Labels
Type: Bug Something isn't working as documented

Comments

@IEvangelist
Copy link
Contributor

What happened?

When I attempt to perform the following REST call using the SDK client, it throws an error:

var tokenProvider = new TokenProvider(Environment.GetEnvironmentVariable("GITHUB_TOKEN") ?? "");
var adapter = RequestAdapter.Create(new TokenAuthProvider(tokenProvider));

var client = new GitHubClient(adapter);

var owner = "DecimalTurn";
var repo = "VBA-on-GitHub-Automations";
var issueCommentId = 2310943199;

try
{
    var issueComment = await client.Repos[owner][repo].Issues.Comments[(int)issueCommentId].GetAsync();
    if (issueComment is { })
    {
        // This is never hit, as the above API always throws.
    }
}
catch (Exception ex)
{
    if (Debugger.IsAttached)
    {
        Debugger.Break();
    }
}       

The error I'm seeing is as follows:

Message: 
GitHub.Models.BasicError : Exception of type 'GitHub.Models.BasicError' was thrown.

  Stack Trace: 
HttpClientRequestAdapter.ThrowIfFailedResponse(HttpResponseMessage response, Dictionary`2 errorMapping, Activity activityForAttributes, CancellationToken cancellationToken)
HttpClientRequestAdapter.SendAsync[ModelType](RequestInformation requestInfo, ParsableFactory`1 factory, Dictionary`2 errorMapping, CancellationToken cancellationToken)
HttpClientRequestAdapter.SendAsync[ModelType](RequestInformation requestInfo, ParsableFactory`1 factory, Dictionary`2 errorMapping, CancellationToken cancellationToken)
WithComment_ItemRequestBuilder.GetAsync(Action`1 requestConfiguration, CancellationToken cancellationToken)
GitHubClientTests.GitHubClientGetsIssueCommentTest() line 45
--- End of stack trace from previous location ---

However, if I run the following code-to manually use an HttpClient outside the library (it works!):

var client = new HttpClient();

client.DefaultRequestHeaders.UserAgent.Add(new("Test", "0.1"));

var token = Environment.GetEnvironmentVariable("GITHUB_TOKEN");
client.DefaultRequestHeaders.Authorization = new("Bearer", token);

var owner = "DecimalTurn";
var repo = "VBA-on-GitHub-Automations";
var issueCommentId = 2310943199;

try
{
    var response = await client.GetAsync($"""
        https://api.github.com/repos/{owner}/{repo}/issues/comments/{issueCommentId}
        """);

    response.EnsureSuccessStatusCode();

    var json = await response.Content.ReadAsStringAsync();
    if (json is { })
    {
        // This gets here, and works with the expected JSON
    }
}
catch (Exception ex)
{
    if (Debugger.IsAttached)
    {
        Debugger.Break();
    }
}

The expected JSON payload is as follows:

Click to expand JSON
{
  "url": "https://api.github.com/repos/DecimalTurn/VBA-on-GitHub-Automations/issues/comments/2310943199",
  "html_url": "https://github.com/DecimalTurn/VBA-on-GitHub-Automations/issues/79#issuecomment-2310943199",
  "issue_url": "https://api.github.com/repos/DecimalTurn/VBA-on-GitHub-Automations/issues/79",
  "id": 2310943199,
  "node_id": "IC_kwDOMhkZW86JvjHf",
  "user": {
    "login": "DecimalTurn",
    "id": 31558169,
    "node_id": "MDQ6VXNlcjMxNTU4MTY5",
    "avatar_url": "https://avatars.githubusercontent.com/u/31558169?u=60e9c89711d781983b36f3701037bb2af138456c&v=4",
    "gravatar_id": "",
    "url": "https://api.github.com/users/DecimalTurn",
    "html_url": "https://github.com/DecimalTurn",
    "followers_url": "https://api.github.com/users/DecimalTurn/followers",
    "following_url": "https://api.github.com/users/DecimalTurn/following{/other_user}",
    "gists_url": "https://api.github.com/users/DecimalTurn/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/DecimalTurn/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/DecimalTurn/subscriptions",
    "organizations_url": "https://api.github.com/users/DecimalTurn/orgs",
    "repos_url": "https://api.github.com/users/DecimalTurn/repos",
    "events_url": "https://api.github.com/users/DecimalTurn/events{/privacy}",
    "received_events_url": "https://api.github.com/users/DecimalTurn/received_events",
    "type": "User",
    "site_admin": false
  },
  "created_at": "2024-08-26T19:39:32Z",
  "updated_at": "2024-08-26T19:39:32Z",
  "author_association": "OWNER",
  "body": "Let's strap on your boots!",
  "reactions": {
    "url": "https://api.github.com/repos/DecimalTurn/VBA-on-GitHub-Automations/issues/comments/2310943199/reactions",
    "total_count": 1,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 1
  },
  "performed_via_github_app": null
}

Versions

  • GitHub.Octokit.SDK v0.0.26
  • .NET v8.0

Code of Conduct

  • I agree to follow this project's Code of Conduct
@IEvangelist IEvangelist added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Aug 28, 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! 🚀

@kfcampbell
Copy link
Member

This is interesting! It's due to an issue in our specification.

What's going on is that issueCommentId is originally defined as an unsigned integer. You're casting it to an integer because that's what Issues.Comments requires, but 2310943199 is larger than the Int32 max value of 2147483647, so we're getting an overflow and what's actually going over the wire is a request for the comment ID of -1984024097. The request is then 404ing, resulting in the error.

The offending integer definition is located at

public global::GitHub.Repos.Item.Item.Issues.Comments.Item.WithComment_ItemRequestBuilder this[int position]
.

The offending JSON schema looks like:

      "comment-id": {
        "name": "comment_id",
        "description": "The unique identifier of the comment.",
        "in": "path",
        "required": true,
        "schema": {
          "type": "integer"
        }
      },

and it resides in github/rest-api-description. This is something we'll have to fix on our backend before the changes can percolate through to the generator.

@kfcampbell kfcampbell moved this from 🆕 Triage to 🔥 Backlog in 🧰 Octokit Active Sep 9, 2024
@github-project-automation github-project-automation bot moved this from 🔥 Backlog to ✅ Done in 🧰 Octokit Active Sep 11, 2024
@IEvangelist
Copy link
Contributor Author

This is something we'll have to fix on our backend before the changes can percolate through to the generator.

Thank you so much, but how can I track when this happens? Also, there have historically been issues between int and long in the Octokit.net SDK. I'm curious if this is something that can be evaluated as a whole to ensure that all values are correct in the API description.

@IEvangelist
Copy link
Contributor Author

@nickfloyd
Copy link
Contributor

nickfloyd commented Nov 14, 2024

Circling back around on this. In our data model there are two ids representing an issue for instance.

One not scoped, which is id that is of type int64 (or long)
GET /issues/{id}
One scoped, which is number that is of type int32 (or int)
GET /repo/{repo_id}/issues/{number}

Are you seeing an issue with the scoped id? I had a look at the DB schema on the back end, I definitely could've missed it but I didn't see instances where these were longs. If it is a problem then we'll need to do another sweep on number as well, just to cover our bases.

Let me know what you are seeing if you get the chance, and sorry for the trouble again.

@nickfloyd nickfloyd reopened this Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
Status: Done
Development

No branches or pull requests

3 participants