Skip to content

Commit

Permalink
better logging on error (#572)
Browse files Browse the repository at this point in the history
* better logging on error
  • Loading branch information
twonirwana authored Sep 29, 2024
1 parent f4bedb6 commit 628d616
Show file tree
Hide file tree
Showing 17 changed files with 70 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,15 @@ protected Collection<CommandDefinitionOption> additionalCommandOptions() {
}
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 Collection<CommandDefinitionOption> additionalCommandOptions() {
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 -> {
BotMetrics.timerAnswerMetricCounter(getCommandId(), stopwatch.elapsed());
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);
}
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 @@ protected Mono<InteractionHook> replyWithEmbedOrMessageDefinition(
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);
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 @@ protected Mono<Void> handleException(@NonNull String errorMessage,
boolean ignoreNotFound) {
switch (throwable) {
case InsufficientPermissionException ignored -> {
log.info("{}: Missing permissions: {} - {}", getGuildAndChannelName(), errorMessage, throwable.getMessage());
log.info("{}: Missing permissions: {} - {}", getErrorRequester(), errorMessage, throwable.getMessage());
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());
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);
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);
}
return Mono.empty();
}
Expand Down Expand Up @@ -264,7 +264,7 @@ protected Optional<String> checkPermission(@NonNull MessageChannel messageChanne
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(),
messageChannel.getName(),
result));
return Optional.of(result);
Expand All @@ -278,7 +278,7 @@ protected Optional<String> checkPermission(@NonNull MessageChannel messageChanne

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

0 comments on commit 628d616

Please sign in to comment.