GitTools / GitReleaseManager

Tool for creating and exporting releases for software applications hosted on GitHub
https://gittools.github.io/GitReleaseManager/docs/
MIT License
298 stars 39 forks source link

Value was either too large or too small for an Int32 #598

Open Jericho opened 1 week ago

Jericho commented 1 week ago

GRM references Octokit 10.0.0 which has known bugs regarding int vs long. We should upgrade to Octokit 12.0.0 which resolves some of these bugs (such as https://github.com/octokit/octokit.net/pull/2928 for instance).

BTW: my guess is that more of these int vs long issues will be discovered and solved in the near future so I wouldn't be surprised if we had to upgrade again in the future.

Here's an example demonstrating that my build is failing due to this issue: https://ci.appveyor.com/project/Jericho/stronggrid/builds/50054134/job/k3i5irek8a2bsa33#L487

Jericho commented 1 week ago

Upgrading Octokit is a good first step but it's not sufficient to resolve this issue. I was able to reproduce this problem even after upgrading the reference. Here's a more detailed stack trace:

 ---> System.OverflowException: Value was either too large or too small for an Int32.
   at System.Convert.ThrowInt32OverflowException()
   at System.Convert.ToInt32(Int64 value)
   at lambda_method117(Closure , Object , IEnumerable`1 , ResolutionContext )
   --- End of inner exception stack trace ---
   at lambda_method117(Closure , Object , IEnumerable`1 , ResolutionContext )
   --- End of inner exception stack trace ---
   at lambda_method117(Closure , Object , IEnumerable`1 , ResolutionContext )
   at GitReleaseManager.Core.Provider.GitHubProvider.<>c__DisplayClass14_0.<<GetIssueCommentsAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at GitReleaseManager.Core.Provider.GitHubProvider.ExecuteAsync[T](Func`1 action)
   --- End of inner exception stack trace ---
   at GitReleaseManager.Core.Provider.GitHubProvider.ExecuteAsync[T](Func`1 action)
   at GitReleaseManager.Core.VcsService.CommentsIncludeStringAsync(String owner, String repository, Issue issue, String comment)
   at GitReleaseManager.Core.VcsService.AddIssueCommentsAsync(String owner, String repository, Milestone milestone)
   at GitReleaseManager.Core.VcsService.CloseMilestoneAsync(String owner, String repository, String milestoneTitle)
   at GitReleaseManager.Core.Commands.CloseCommand.ExecuteAsync(CloseSubOptions options)
   at GitReleaseManager.Cli.Program.Main(String[] args)

I notice that stack trace mentions GetIssueCommentsAsync which, as its name implies, retrieves comments for a given issue. So I investigated al code and classes related to this functinoality and noticed the model for a comment is defined in GRM with the following property:

public class IssueComment
{
    /// <summary>
    /// Gets or sets the issue comment Id.
    /// </summary>
    public int Id { get; set; }

Probably should change the type of the Id property to match the type of the data returned from the GitHub API.

I scanned all the model classes and noticed a few more properties currently defined as int which should probably be changed to long:

Jericho commented 1 week ago

After further investigation it appears that Octokit still defines Release.Id and ReleaseAsset.Id as int therefore we shouldn't change these two properties in our model classes.

Jericho commented 1 week ago

I can confirm that after upgrading the Octokit reference and changing IssueComment.Id to long, my build conpletes successfully

========================================
Publish-GitHub-Release
========================================

   ____ _ _   ____      _                     __  __
  / ___(_) |_|  _ \ ___| | ___  __ _ ___  ___|  \/  | __ _ _ __   __ _  __ _  ___ _ __
 | |  _| | __| |_) / _ \ |/ _ \/ _` / __|/ _ \ |\/| |/ _` | '_ \ / _` |/ _` |/ _ \ '__|
 | |_| | | |_|  _ <  __/ |  __/ (_| \__ \  __/ |  | | (_| | | | | (_| | (_| |  __/ |
  \____|_|\__|_| \_\___|_|\___|\__,_|___/\___|_|  |_|\__,_|_| |_|\__,_|\__, |\___|_|
                                                                       |___/
                                                               0.17.0-collaborators0001

[VRB] Loading configuration from file: D:\_build\StrongGrid\GitReleaseManager.yaml
[VRB] Successfully deserialized configuration!
Using GitHub as VCS Provider
Closing milestone 0.109.0
[VRB] Finding open milestone with title '0.109.0' on 'Jericho/StrongGrid'
[VRB] Closing milestone '0.109.0' on 'Jericho/StrongGrid'
[VRB] Finding issues with milestone: '127
[VRB] Finding issue comment created by GitReleaseManager for issue #527
Issue #527 already contains release comment, skipping...
[VRB] Finding issue comment created by GitReleaseManager for issue #526
Adding release comment for Issue #526
[VRB] Finding issue comment created by GitReleaseManager for issue #525
Adding release comment for Issue #525
[VRB] Finding issue comment created by GitReleaseManager for issue #524
Adding release comment for Issue #524
[VRB] Finding issue comment created by GitReleaseManager for issue #523
Adding release comment for Issue #523

----------------------------------------
TearDown
----------------------------------------
Finished running tasks.

Task                          Duration
--------------------------------------------------
Setup                         00:00:00.4135320
Publish-GitHub-Release        00:00:03.6450926
Teardown                      00:00:00.0009156
--------------------------------------------------
Total:                        00:00:04.0595402
Jericho commented 1 week ago

Submitted a PR with necessary changes. I did not change Milestone.InternalNumber to long since it's not needed to resolve this issue but I'll be happy to amend my PR if the consensus is that we should proactively change it as well.