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

NPE when updating comment in issue #1218

Closed
markwoon opened this issue Aug 30, 2021 · 5 comments
Closed

NPE when updating comment in issue #1218

markwoon opened this issue Aug 30, 2021 · 5 comments
Labels

Comments

@markwoon
Copy link

I'm getting an NPE when trying to update a comment in an issue:

java.lang.NullPointerException: null
	at org.kohsuke.github.GHIssueComment.getApiRoute(GHIssueComment.java:149)
	at org.kohsuke.github.GHIssueComment.update(GHIssueComment.java:114)
	...

The NPE is because owner.getRepository() is returning null.

I can update the issue's body just fine, but cannot update issue comments.

This is with 1.132.

@bitwiseman
Copy link
Member

@markwoon What method are you using to get the GHIssueComment that is throwing?

@markwoon
Copy link
Author

Something along the lines of:

GitHub github = new GitHubBuilder()
  .withOAuthToken(token)
  .build();
GHRepository repo = github.getRepository(repoName);

GHIssueSearchBuilder builder = github.searchIssues()
  .isOpen()
  .q("foo")
  .q("repo:" + repo.getFullName());
GHIssue issue = builder.list.get(0);

for (GHIssueComment comment : issue.listComments()) {
  if (!comment.getBody().contains("*THIS ISSUE HAS BEEN UPDATED!*")) {
    comment.update(addUpdatedText(comment.getBody()));
  }
}

@bitwiseman
Copy link
Member

Thought it was something like that.

This doesn't set the owner on GHIssue:

GHIssue[] getItems(GitHub root) {
for (GHIssue i : items)
i.wrap(root);
return items;
}

There's several things that should be fixed here:

  • GHIssueComment.getApiRoute() should call GHIssue.getIssuesApiRoute() instead of building itself GHIssue.getIssuesApiRoute() will need to be made package-private for this to work.
  • GHIssue.listEvents() should be updated to call GHIssue.getIssuesApiRoute() instead of referencing owner.
  • GHIssue.owner and GHIssue.getRepository() should have an annotation and a comment like this one added indicating it maybe null and to use getIssuesApiRoute() instead.
  • Perhaps GHIssue.getRepository() should check if owner is null and populate it if needed? However, that could result in extra requests. The alternative is another annotation and a comment like on owner, or at least a null check with a message similar to that comment so users know what is going.

@bitwiseman bitwiseman added the bug label Sep 2, 2021
@bitwiseman
Copy link
Member

bitwiseman commented Oct 21, 2021

  • Perhaps GHIssue.getRepository() should check if owner is null and populate it if needed? However, that could result in extra requests. The alternative is another annotation and a comment like on owner, or at least a null check with a message similar to that comment so users know what is going.

This is what #1260 implemented. Released in v1.135.
I think that fixes this issue. @markwoon Can you check this?
We'll still need a test before closing this issue though.

@markwoon
Copy link
Author

Confirming fixed for my use case in v1.135!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants