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

better logging on error #572

Merged
merged 3 commits into from
Sep 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,15 @@ protected MessageDataDTO createEmptyMessageData(@NonNull UUID configUUID,
}
final Locale userOrConfigLocale = BaseCommandOptions.getLocaleOptionFromStartCommandOption(options)
.orElse(event.getRequester().getUserLocal());
Optional<String> validationMessage = getStartOptionsValidationMessage(options, event.getChannelId(), event.getUserId(), userOrConfigLocale);
Optional<String> validationMessage = getStartOptionsValidationMessage(options, event.getChannelId(), event.getUserId(), Optional.ofNullable(userOrConfigLocale).orElse(Locale.ENGLISH));
if (validationMessage.isPresent()) {
log.info("{}: Validation message: {} for {}", event.getRequester().toLogString(),
validationMessage.get().replace("\n", " "),
commandString.replace("\n", " "));
//todo i18n?
return event.reply(String.format("%s\n%s", commandString, validationMessage.get()), true);
}
final C config = getConfigFromStartOptions(options, userOrConfigLocale);
final C config = getConfigFromStartOptions(options, Optional.ofNullable(userOrConfigLocale).orElse(Locale.ENGLISH));
final UUID configUUID = uuidSupplier.get();
BotMetrics.incrementSlashStartMetricCounter(getCommandId());

Expand All @@ -171,7 +171,7 @@ protected MessageDataDTO createEmptyMessageData(@NonNull UUID configUUID,
commandString.replace("`", "").replace("\n", " "));
String replayMessage = Stream.of(commandString, getConfigWarnMessage(config, userLocale).orElse(null))
.filter(s -> !Strings.isNullOrEmpty(s))
.collect(Collectors.joining(" "));
.collect(Collectors.joining("\n"));

return event.reply(replayMessage, false)
.then(Mono.defer(() -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import java.util.function.Supplier;

@Value
@Builder
@Builder(toBuilder = true)
public class RollAnswer {
@NonNull
AnswerFormatType answerFormatType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ private List<ComponentRowDefinition> createButtonLayout(UUID configUUID, CustomD
})
.filter(s -> !Strings.isNullOrEmpty(s))
.distinct()
.collect(Collectors.joining(", "))));
.collect(Collectors.joining("\n"))));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Stopwatch;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import de.janno.discord.bot.AnswerInteractionType;
import de.janno.discord.bot.BotMetrics;
import de.janno.discord.bot.I18n;
Expand Down Expand Up @@ -56,6 +58,22 @@ public DirectRollCommand(PersistenceManager persistenceManager, CachingDiceEvalu
this.expressionOptionName = expressionOptionName;
}

private static EmbedOrMessageDefinition createAnswerWithOptionalWarning(RollAnswer answer) {
EmbedOrMessageDefinition answerMessage = RollAnswerConverter.toEmbedOrMessageDefinition(answer);
if (!Strings.isNullOrEmpty(answer.getWarning()) &&
answerMessage.getType() == EmbedOrMessageDefinition.Type.EMBED
&& answerMessage.getFields().size() < 25) {
answerMessage = answerMessage.toBuilder()
.fields(ImmutableList.<EmbedOrMessageDefinition.Field>builder()
.addAll(answerMessage.getFields())
.add(new EmbedOrMessageDefinition.Field("Warning", answer.getWarning(), false))
.build()
)
.build();
}
return answerMessage;
}

@Override
public @NonNull String getCommandId() {
return ROLL_COMMAND_ID;
Expand Down Expand Up @@ -153,8 +171,9 @@ protected EmbedOrMessageDefinition getHelpMessage(Locale userLocale) {
@NonNull Stopwatch stopwatch,
@NonNull Locale userLocale) {

//ignore warning, no good way to display it
Mono<Void> answerMono = Mono.defer(() -> event.replyWithEmbedOrMessageDefinition(RollAnswerConverter.toEmbedOrMessageDefinition(answer), false));
final EmbedOrMessageDefinition answerMessage = createAnswerWithOptionalWarning(answer);

Mono<Void> answerMono = Mono.defer(() -> event.replyWithEmbedOrMessageDefinition(answerMessage, false));
return answerMono
.doOnSuccess(v ->
log.info("{}: '{}'={} -> {} in {}ms",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public Mono<Void> editMessage(@Nullable String message, @Nullable List<Component

@Override
public Requester getRequester() {
return new Requester("invokingUser", "channelName", "guildName", "[0 / 1]", Locale.ENGLISH);
return new Requester("invokingUser", "channelName", "guildName", "[0 / 1]", Locale.ENGLISH, null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public String getCommandString() {

@Override
public Requester getRequester() {
return new Requester("invokingUser", "channelName", "guildName", "[0 / 1]", userLocale);
return new Requester("invokingUser", "channelName", "guildName", "[0 / 1]", userLocale, null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ de.janno.discord.bot.command.customDice.CustomDiceCommandMockTest.slash_start_mu

de.janno.discord.bot.command.customDice.CustomDiceCommandMockTest.slash_start_warn=[
[
"reply: commandString `3`: did not contain any random element, try for Example `d20` to roll a 20 sided die, `'a'`: did not contain any random element, try for Example `d20` to roll a 20 sided die",
"reply: commandString\n`3`: did not contain any random element, try for Example `d20` to roll a 20 sided die\n`'a'`: did not contain any random element, try for Example `d20` to roll a 20 sided die",
"sendMessage: EmbedOrMessageDefinition(title=null, descriptionOrContent=Click on a button to roll the dice, fields=[], componentRowDefinitions=[ComponentRowDefinition(buttonDefinitions=[ButtonDefinition(label=1d6, id=custom_dice\u001E1_button\u001E00000000-0000-0000-0000-000000000000, style=PRIMARY, disabled=false, emoji=null), ButtonDefinition(label=Attack, id=custom_dice\u001E2_button\u001E00000000-0000-0000-0000-000000000000, style=PRIMARY, disabled=false, emoji=null), ButtonDefinition(label=3d10,3d10,3d10, id=custom_dice\u001E3_button\u001E00000000-0000-0000-0000-000000000000, style=PRIMARY, disabled=false, emoji=null), ButtonDefinition(label=3, id=custom_dice\u001E4_button\u001E00000000-0000-0000-0000-000000000000, style=PRIMARY, disabled=false, emoji=null), ButtonDefinition(label='a', id=custom_dice\u001E5_button\u001E00000000-0000-0000-0000-000000000000, style=PRIMARY, disabled=false, emoji=null)])], hasImage=false, type=MESSAGE, userReference=false, sendToOtherChannelId=null)"
]
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ void handleSlashCommandEvent() {

when(event.sendMessage(any())).thenReturn(Mono.just(2L));
when(event.deleteMessageById(anyLong())).thenReturn(Mono.empty());
when(event.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH));
when(event.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH, null));
when(event.reply(any(), anyBoolean())).thenReturn(Mono.just(mock(Void.class)));

Mono<Void> res = underTest.handleSlashCommandEvent(event, () -> UUID.fromString("00000000-0000-0000-0000-000000000000"), Locale.ENGLISH);
Expand Down Expand Up @@ -285,7 +285,7 @@ void handleSlashCommandEvent_help() {
when(event.getOption("help")).thenReturn(Optional.of(CommandInteractionOption.builder()
.name("help")
.build()));
when(event.getRequester()).thenReturn(new Requester("userName", "channelName", "guildName", "shard 1/1", Locale.ENGLISH));
when(event.getRequester()).thenReturn(new Requester("userName", "channelName", "guildName", "shard 1/1", Locale.ENGLISH, null));
when(event.replyWithEmbedOrMessageDefinition(any(), anyBoolean())).thenReturn(Mono.just(mock(Void.class)));

Mono<Void> res = underTest.handleSlashCommandEvent(event, () -> UUID.fromString("00000000-0000-0000-0000-000000000000"), Locale.ENGLISH);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ de.janno.discord.bot.command.directRoll.DirectRollCommandMockTest.roll_multiLine

de.janno.discord.bot.command.directRoll.DirectRollCommandMockTest.roll_warn=[
[
"replyWithEmbedOrMessageDefinition: EmbedOrMessageDefinition(title=20 ⇒ 20, descriptionOrContent=, fields=[], componentRowDefinitions=[], hasImage=false, type=EMBED, userReference=true, sendToOtherChannelId=null)"
"replyWithEmbedOrMessageDefinition: EmbedOrMessageDefinition(title=20 ⇒ 20, descriptionOrContent=, fields=[EmbedOrMessageDefinition.Field(name=Warning, value=did not contain any random element, try for Example `d20` to roll a 20 sided die, inline=false)], componentRowDefinitions=[], hasImage=false, type=EMBED, userReference=true, sendToOtherChannelId=null)"
]
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void handleComponentInteractEvent_validationFailed() {
when(slashEventAdaptor.getOption(any())).thenReturn(Optional.of(interactionOption));
when(slashEventAdaptor.reply(any(), anyBoolean())).thenReturn(Mono.just(mock(Void.class)));
when(slashEventAdaptor.getCommandString()).thenReturn("/r expression:asdfasdf");
when(slashEventAdaptor.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH));
when(slashEventAdaptor.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH, null));

Mono<Void> res = underTest.handleSlashCommandEvent(slashEventAdaptor, () -> UUID.fromString("00000000-0000-0000-0000-000000000000"), Locale.ENGLISH);

Expand Down Expand Up @@ -97,7 +97,7 @@ void handleComponentInteractEvent_help() {
when(slashEventAdaptor.getChannelId()).thenReturn(1L);
when(slashEventAdaptor.replyWithEmbedOrMessageDefinition(any(), anyBoolean())).thenReturn(Mono.just(mock(Void.class)));
when(slashEventAdaptor.getCommandString()).thenReturn("/r expression:help");
when(slashEventAdaptor.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH));
when(slashEventAdaptor.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH, null));


Mono<Void> res = underTest.handleSlashCommandEvent(slashEventAdaptor, () -> UUID.fromString("00000000-0000-0000-0000-000000000000"), Locale.ENGLISH);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void handleComponentInteractEvent_validationFailed() {
when(slashEventAdaptor.getOption(any())).thenReturn(Optional.of(interactionOption));
when(slashEventAdaptor.reply(any(), anyBoolean())).thenReturn(Mono.just(mock(Void.class)));
when(slashEventAdaptor.getCommandString()).thenReturn("/r expression:asdfasdf");
when(slashEventAdaptor.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH));
when(slashEventAdaptor.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH, null));

Mono<Void> res = underTest.handleSlashCommandEvent(slashEventAdaptor, () -> UUID.fromString("00000000-0000-0000-0000-000000000000"), Locale.ENGLISH);

Expand Down Expand Up @@ -120,7 +120,7 @@ void handleComponentInteractEvent_help() {
when(slashEventAdaptor.getChannelId()).thenReturn(1L);
when(slashEventAdaptor.replyWithEmbedOrMessageDefinition(any(), anyBoolean())).thenReturn(Mono.just(mock(Void.class)));
when(slashEventAdaptor.getCommandString()).thenReturn("/r expression:help");
when(slashEventAdaptor.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH));
when(slashEventAdaptor.getRequester()).thenReturn(new Requester("user", "channel", "guild", "[0 / 1]", Locale.ENGLISH, null));


Mono<Void> res = underTest.handleSlashCommandEvent(slashEventAdaptor, () -> UUID.fromString("00000000-0000-0000-0000-000000000000"), Locale.ENGLISH);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,32 @@

import lombok.Value;

import javax.annotation.Nullable;
import java.util.Locale;
import java.util.Optional;
import java.util.UUID;

@Value
public class Requester {
String userName;
@Nullable
String channelName;
@Nullable
String guildName;
String shard;
@Nullable
Locale userLocal;

@Nullable
UUID configUUID;

public String toLogString() {
return Optional.ofNullable(guildName)
String name = Optional.ofNullable(guildName)
.or(() -> Optional.ofNullable(channelName))
.or(() -> Optional.ofNullable(userName))
.orElse("");
if (configUUID != null) {
return "%s:%s".formatted(configUUID, name);

Check warning on line 29 in discord-connector/api/src/main/java/de/janno/discord/connector/api/Requester.java

View check run for this annotation

Codecov / codecov/patch

discord-connector/api/src/main/java/de/janno/discord/connector/api/Requester.java#L29

Added line #L29 was not covered by tests
}
return name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public Optional<String> checkPermissions(Long answerTargetChannelId, @NonNull Lo
}

@Override
protected @NonNull String getGuildAndChannelName() {
protected @NonNull String getErrorRequester() {
return requester.toLogString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@
switch (messageDefinition.getType()) {
case EMBED -> {
//reply don't need avatar and name, they are already displayed
EmbedBuilder builder = convertToEmbedMessage(messageDefinition, null, null, rollRequesterId);
EmbedBuilder builder = convertToEmbedMessage(messageDefinition, null, null, rollRequesterId);

Check warning on line 148 in discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java

View check run for this annotation

Codecov / codecov/patch

discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java#L148

Added line #L148 was not covered by tests
final List<FileUpload> files = applyFiles(builder, messageDefinition);
return createMonoFrom(() -> event.replyEmbeds(builder.build()).setComponents(layoutComponents).setEphemeral(ephemeral).setFiles(files).setSuppressedNotifications(true))
.onErrorResume(t -> handleException("Error on replay embed message", t, false).ofType(InteractionHook.class));
Expand All @@ -172,23 +172,23 @@
boolean ignoreNotFound) {
switch (throwable) {
case InsufficientPermissionException ignored -> {
log.info("{}: Missing permissions: {} - {}", getGuildAndChannelName(), errorMessage, throwable.getMessage());
log.info("{}: Missing permissions: {} - {}", getErrorRequester(), errorMessage, throwable.getMessage());

Check warning on line 175 in discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java

View check run for this annotation

Codecov / codecov/patch

discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java#L175

Added line #L175 was not covered by tests
return Mono.empty();
}
case ErrorResponseException ignored when ((ErrorResponseException) throwable).getErrorResponse() == ErrorResponse.MISSING_PERMISSIONS -> {
log.info("{}: Missing permissions: {} - {}", getGuildAndChannelName(), errorMessage, throwable.getMessage());
log.info("{}: Missing permissions: {} - {}", getErrorRequester(), errorMessage, throwable.getMessage());

Check warning on line 179 in discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java

View check run for this annotation

Codecov / codecov/patch

discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java#L179

Added line #L179 was not covered by tests
return Mono.empty();
}
//we can check if the emoji exists, so the error is simply cached
case ErrorResponseException ignored when ((ErrorResponseException) throwable).getSchemaErrors().stream()
.flatMap(se -> se.getErrors().stream())
.anyMatch(e -> "BUTTON_COMPONENT_INVALID_EMOJI".equals(e.getCode())) -> {
log.info("{}: Invalid emoji: {}", getGuildAndChannelName(), errorMessage);
log.info("{}: Invalid emoji: {}", getErrorRequester(), errorMessage);

Check warning on line 186 in discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java

View check run for this annotation

Codecov / codecov/patch

discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java#L186

Added line #L186 was not covered by tests
return Mono.empty();
}
case ErrorResponseException ignored when ((ErrorResponseException) throwable).getErrorResponse().getCode() < 20000 && ignoreNotFound ->
log.trace("{}: Not found: {}", getGuildAndChannelName(), errorMessage);
default -> log.error("{}: {}", getGuildAndChannelName(), errorMessage, throwable);
log.trace("{}: Not found: {}", getErrorRequester(), errorMessage);
default -> log.error("{}: {}", getErrorRequester(), errorMessage, throwable);

Check warning on line 191 in discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java

View check run for this annotation

Codecov / codecov/patch

discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java#L190-L191

Added lines #L190 - L191 were not covered by tests
}
return Mono.empty();
}
Expand Down Expand Up @@ -264,7 +264,7 @@
String result = I18n.getMessage("permission.missing", userLocale, String.join(", ", checks));

log.info(String.format("'%s'.'%s': %s",
Optional.of(guild).map(Guild::getName).orElse("-"),
getErrorRequester(),

Check warning on line 267 in discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java

View check run for this annotation

Codecov / codecov/patch

discord-connector/jda/src/main/java/de/janno/discord/connector/jda/DiscordAdapterImpl.java#L267

Added line #L267 was not covered by tests
messageChannel.getName(),
result));
return Optional.of(result);
Expand All @@ -278,7 +278,7 @@

protected abstract @NonNull MessageChannel getMessageChannel();

protected abstract @NonNull String getGuildAndChannelName();
protected abstract @NonNull String getErrorRequester();

protected @NonNull ParallelFlux<MessageState> getMessagesState(@NonNull MessageChannel messageChannel, @NonNull Collection<Long> messageIds) {
return Flux.fromIterable(messageIds)
Expand Down
Loading
Loading