search

Together-Java / TJ-Bot

TJ-Bot is a Discord Bot used on the Together Java server. It is maintained by the community, anyone can contribute.
https://togetherjava.org
GNU General Public License v3.0
100 stars 87 forks source link

replace getAsTag() with getName() on user entity #1016

Closed ankitsmt211 closed 7 months ago

ankitsmt211 commented 8 months ago

replaces User#getAsTag() with User#getName() throughout code, except RemindRoutine.java, RemindRoutineTest.java(facing some issues in two tests from RemindRoutineTest) and FilesharingListener.java (already active PR)

ankitsmt211 commented 8 months ago

@Zabuzard i see you wrote RemindRoutineTest.java

This is causing some trouble and i have no clue, basically replacing getAsTag() in RemindRoutine.java

with getName() in both files triggers an error MessageEmbed#getAuthor is null so can't call getName() on it.

Perhaps you have some insight ?

Zabuzard commented 8 months ago

@Zabuzard i see you wrote RemindRoutineTest.java

This is causing some trouble and i have no clue, basically replacing getAsTag() in RemindRoutine.java

with getName() in both files triggers an error MessageEmbed#getAuthor is null so can't call getName() on it.

Perhaps you have some insight ?

Kindly commit and push the change, so I can see the full log with the exact failed test and everything in the CI/CD logs, thank you.

ankitsmt211 commented 8 months ago

@Zabuzard these commits contain the problematic changes i was talking about

Zabuzard commented 8 months ago

@ankitsmt211 the error message is

RemindRoutineTest > Sends out a pending reminder to a guild channel, the base case FAILED java.lang.NullPointerException at RemindRoutineTest.java:105

RemindRoutineTest > Sends out a pending reminder via DM, even if the channel could not be retrieved anymore FAILED java.lang.NullPointerException at RemindRoutineTest.java:148

Now, unfortunately I am lacking the full stacktrace. So if you would please run the test manually and send the full stacktrace, that would be awesome 👍

ankitsmt211 commented 8 months ago

@ankitsmt211 the error message is

RemindRoutineTest > Sends out a pending reminder to a guild channel, the base case FAILED java.lang.NullPointerException at RemindRoutineTest.java:105

RemindRoutineTest > Sends out a pending reminder via DM, even if the channel could not be retrieved anymore FAILED java.lang.NullPointerException at RemindRoutineTest.java:148

Now, unfortunately I am lacking the full stacktrace. So if you would please run the test manually and send the full stacktrace, that would be awesome 👍

@Zabuzard Please have a look


java.lang.NullPointerException: Cannot invoke "okhttp3.OkHttpClient.newCall(okhttp3.Request)" because "this.httpClient" is null
    at net.dv8tion.jda.internal.requests.Requester.execute(Requester.java:180) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.internal.requests.Requester.execute(Requester.java:143) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.internal.requests.Requester.execute(Requester.java:126) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.internal.requests.ratelimit.BotRateLimiter$Bucket.run(BotRateLimiter.java:481) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572) ~[?:?]
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317) ~[?:?]
    at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) ~[?:?]
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[?:?]
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[?:?]
    at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]
13:40:27.078 [pool-3-thread-1] ERROR net.dv8tion.jda.internal.requests.RateLimiter - Encountered exception trying to execute request
java.lang.NullPointerException: Cannot invoke "java.util.concurrent.ExecutorService.execute(java.lang.Runnable)" because the return value of "net.dv8tion.jda.internal.JDAImpl.getCallbackPool()" is null
    at net.dv8tion.jda.api.requests.Request.onFailure(Request.java:150) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.api.requests.Request.onFailure(Request.java:139) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.internal.requests.RestActionImpl.handleResponse(RestActionImpl.java:284) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.api.requests.Request.handleResponse(Request.java:282) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.internal.requests.Requester.execute(Requester.java:242) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.internal.requests.Requester.execute(Requester.java:143) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.internal.requests.Requester.execute(Requester.java:126) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at net.dv8tion.jda.internal.requests.ratelimit.BotRateLimiter$Bucket.run(BotRateLimiter.java:481) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572) ~[?:?]
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317) ~[?:?]
    at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) ~[?:?]
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[?:?]
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[?:?]
    at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]

java.lang.NullPointerException: Cannot invoke "net.dv8tion.jda.api.entities.MessageEmbed$AuthorInfo.getName()" because the return value of "net.dv8tion.jda.api.entities.MessageEmbed.getAuthor()" is null
    at org.togetherjava.tjbot.features.reminder.RemindRoutineTest.sendsPendingReminderChannelFoundAuthorFound(RemindRoutineTest.java:105)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)```
Zabuzard commented 8 months ago

@ankitsmt211 thanks, fixed.

Let me quickly explain. getAuthor returned null, why is that the case? It obviously has to do with the change you did in RemindRoutine. So you set a breakpoint there and notice that author.getName() in this line here:

String authorName = author == null ? "Unknown user" : author.getName();

returned null. So authorName = null. This is then set in the embed:

EmbedBuilder().setAuthor(authorName, null, authorIconUrl)

An embed with null as authorName leads to the embed.getAuthor() later in the test to return null consequently.

So now you have to find out why author.getName() was null. Thats because we do not use "real users" here, but mocks. So you go to where this user comes from, namely JdaTester:

UserImpl user = spy(new UserImpl(USER_ID, jda));

Thats the user used on all those messages. Now, if you look into UserImpl (from JDA), you see its getName() returning a field thats by default null and can be set via setName. So adding that and it works.

In practice, outside of tests, all those fields are populated by the real JDA when receiving Discord API responses etc. But during tests, we have to mock what JDA does ourselves. And this is (obviously) only done to the extend we actually needed. And you just went beyond what was already covered with your change - our mock users had no names yet.

ankitsmt211 commented 8 months ago

@ankitsmt211 thanks, fixed.

Let me quickly explain. getAuthor returned null, why is that the case? It obviously has to do with the change you did in RemindRoutine. So you set a breakpoint there and notice that author.getName() in this line here:

String authorName = author == null ? "Unknown user" : author.getName();

returned null. So authorName = null. This is then set in the embed:

EmbedBuilder().setAuthor(authorName, null, authorIconUrl)

An embed with null as authorName leads to the embed.getAuthor() later in the test to return null consequently.

So now you have to find out why author.getName() was null. Thats because we do not use "real users" here, but mocks. So you go to where this user comes from, namely JdaTester:

UserImpl user = spy(new UserImpl(USER_ID, jda));

Thats the user used on all those messages. Now, if you look into UserImpl (from JDA), you see its getName() returning a field thats by default null and can be set via setName. So adding that and it works.

In practice, outside of tests, all those fields are populated by the real JDA when receiving Discord API responses etc. But during tests, we have to mock what JDA does ourselves. And this is (obviously) only done to the extend we actually needed. And you just went beyond what was already covered with your change - our mock users had no names yet.

i think get the idea, by default all mocked values are null unless specified. But i couldn't track down the source where mocked user was setup, thanks for resolving the issue.