Skip to content

Commit

Permalink
I18N-132 - Mojito CLI failures should trigger an incident
Browse files Browse the repository at this point in the history
An incident is triggered when a Mojito CLI command fails
  • Loading branch information
DarKhaos committed Oct 2, 2024
1 parent f57622e commit c1c7e22
Show file tree
Hide file tree
Showing 13 changed files with 258 additions and 98 deletions.
11 changes: 11 additions & 0 deletions cli/src/main/java/com/box/l10n/mojito/cli/command/Command.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ public abstract class Command {
description = HELP_DESCRIPTION)
private boolean help;

@Parameter(
names = {"--failure-slack-notification-channel", "-fsnc"},
arity = 1,
description =
"Slack channel to which notifications are sent, required if sending Slack notifications when CLI fails.")
private String failureSlackNotificationChannel;

/**
* Method to be overridden to implement the business logic of this command
*
Expand Down Expand Up @@ -114,4 +121,8 @@ public String getName() {
public void setOriginalArgs(List<String> originalArgs) {
this.originalArgs = originalArgs;
}

public String getFailureSlackNotificationChannel() {
return this.failureSlackNotificationChannel;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
*/
public class CommandException extends RuntimeException {

private boolean expected;

public CommandException(Throwable cause) {
super(cause);
}
Expand All @@ -22,4 +24,13 @@ public CommandException(String message) {
public CommandException(String message, Throwable cause) {
super(message, cause);
}

public CommandException(String message, boolean expected) {
super(message);
this.expected = expected;
}

public boolean isExpected() {
return this.expected;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ private void checkForHardFail(List<CliCheckResult> results) {
sendFailureNotifications(
results.stream().filter(result -> !result.isSuccessful()).collect(Collectors.toList()),
true);
throw new CommandException(cliFailureString);
throw new CommandException(cliFailureString, true);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,19 @@
import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters;
import com.box.l10n.mojito.cli.command.extraction.AssetExtractionDiff;
import com.box.l10n.mojito.cli.command.extraction.ExtractionDiffNotificationSender;
import com.box.l10n.mojito.cli.command.extraction.ExtractionDiffPaths;
import com.box.l10n.mojito.cli.command.extraction.ExtractionDiffService;
import com.box.l10n.mojito.cli.command.extraction.ExtractionDiffStatistics;
import com.box.l10n.mojito.cli.command.extraction.ExtractionPaths;
import com.box.l10n.mojito.cli.command.extraction.MissingExtractionDirectoryException;
import com.box.l10n.mojito.cli.command.param.Param;
import com.box.l10n.mojito.cli.command.utils.SlackNotificationSender;
import com.box.l10n.mojito.cli.console.ConsoleWriter;
import com.box.l10n.mojito.json.ObjectMapper;
import com.box.l10n.mojito.rest.entity.Repository;
import com.box.l10n.mojito.rest.entity.SourceAsset;
import com.box.l10n.mojito.shell.Shell;
import com.box.l10n.mojito.slack.SlackClient;
import com.google.common.base.Strings;
import java.nio.file.Path;
import java.util.Collections;
Expand Down Expand Up @@ -237,12 +238,15 @@ public class ExtractionDiffCommand extends Command {

@Autowired PushService pushService;

private Optional<ExtractionDiffNotificationSender> notificationSender;
@Autowired(required = false)
private SlackClient slackClient;

private Optional<SlackNotificationSender> notificationSender;

// Method for testing purposes
protected Optional<ExtractionDiffNotificationSender> getNotificationSender() {
protected Optional<SlackNotificationSender> getNotificationSender() {
if (!Strings.isNullOrEmpty(this.slackNotificationChannel)) {
return of(new ExtractionDiffNotificationSender(this.slackNotificationChannel));
return of(new SlackNotificationSender(this.slackClient));
}
return empty();
}
Expand Down Expand Up @@ -271,6 +275,7 @@ private void checkMaxStringsBlock(ExtractionDiffPaths extractionDiffPaths)
this.notificationSender.ifPresent(
notificationSender ->
notificationSender.sendMessage(
this.slackNotificationChannel,
String.format(
MAX_STRINGS_ADDED_BLOCK_MESSAGE,
this.pushToBranchName,
Expand All @@ -284,6 +289,7 @@ private void checkMaxStringsBlock(ExtractionDiffPaths extractionDiffPaths)
this.notificationSender.ifPresent(
notificationSender ->
notificationSender.sendMessage(
this.slackNotificationChannel,
String.format(
MAX_STRINGS_REMOVED_BLOCK_MESSAGE,
this.pushToBranchName,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.box.l10n.mojito.cli.command;

import com.beust.jcommander.JCommander;
import com.box.l10n.mojito.cli.command.utils.SlackNotificationSender;
import com.box.l10n.mojito.cli.console.ConsoleWriter;
import com.box.l10n.mojito.rest.resttemplate.AuthenticatedRestTemplate;
import com.box.l10n.mojito.slack.SlackClient;
import com.google.common.base.Strings;
import jakarta.annotation.PostConstruct;
import java.util.Arrays;
Expand Down Expand Up @@ -43,6 +45,9 @@ public class L10nJCommander {

@Autowired AuthenticatedRestTemplate authenticatedRestTemplate;

@Autowired(required = false)
SlackClient slackClient;

static final String PROGRAM_NAME = "mojito";

boolean systemExitEnabled = true;
Expand Down Expand Up @@ -92,6 +97,24 @@ public void init() {
createJCommanderForRun();
}

private void notifyFailure(Command command, String[] args, String errorMessage) {
if (!Strings.isNullOrEmpty(command.getFailureSlackNotificationChannel())) {
new SlackNotificationSender(this.slackClient)
.sendMessage(
command.getFailureSlackNotificationChannel(),
String.format(
":warning: The following command has failed: *%s*\n\n*ARGUMENTS:*\n%s\n\n*ERROR MESSAGE:*\n%s",
command.getName(), String.join(", ", args), errorMessage));
}
}

private void notifyFailure(Command command, String[] args, Throwable throwable) {
this.notifyFailure(
command,
args,
String.format("%s\n%s", throwable.getMessage(), ExceptionUtils.getStackTrace(throwable)));
}

public void run(String... args) {

Exception parsingException = null;
Expand Down Expand Up @@ -124,21 +147,30 @@ public void run(String... args) {
try {
command.run();
} catch (SessionAuthenticationException ae) {
String message = "Invalid username or password";
logger.debug("Exit with Invalid username or password", ae);
printErrorMessage("Invalid username or password");
printErrorMessage(message);
this.notifyFailure(command, args, message);
exitWithError();
} catch (CommandWithExitStatusException cwese) {
logger.error("Exit with error for command: " + command.getName(), cwese);
String message = "Exit with error for command: " + command.getName();
logger.error(message, cwese);
this.notifyFailure(command, args, cwese);
exitWithError(cwese.getExitCode());
} catch (CommandException ce) {
String message = "Exit with error for command: " + command.getName();
printErrorMessage(ce.getMessage());
logger.error("Exit with error for command: " + command.getName(), ce);
logger.error(message, ce);
if (!ce.isExpected()) {
this.notifyFailure(command, args, ce);
}
exitWithError();
} catch (ResourceAccessException rae) {
String msg =
"Is a server running on: " + authenticatedRestTemplate.getURIForResource("") + "?";
printErrorMessage(msg);
logger.error(msg, rae);
this.notifyFailure(command, args, msg);
exitWithError();
} catch (HttpClientErrorException | HttpServerErrorException e) {
String msg =
Expand All @@ -151,11 +183,13 @@ public void run(String... args) {
printErrorMessage(msg);
logger.error("Unexpected error", e);
logger.error(e.getResponseBodyAsString());
this.notifyFailure(command, args, msg);
exitWithError();
} catch (Throwable t) {
String msg = "Unexpected error: " + t.getMessage() + "\n" + ExceptionUtils.getStackTrace(t);
printErrorMessage(msg);
logger.error("Unexpected error", t);
this.notifyFailure(command, args, msg);
exitWithError();
}
}
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.box.l10n.mojito.cli.command.utils;

import static com.box.l10n.mojito.slack.SlackClient.COLOR_WARNING;
import static com.box.l10n.mojito.slack.request.Attachment.MRKDWNIN_TEXT;

import com.box.l10n.mojito.slack.SlackClient;
import com.box.l10n.mojito.slack.SlackClientException;
import com.box.l10n.mojito.slack.request.Attachment;
import com.box.l10n.mojito.slack.request.Message;
import com.google.common.base.Strings;

public class SlackNotificationSender {
private final SlackClient slackClient;

public SlackNotificationSender(SlackClient slackClient) {
this.slackClient = slackClient;
}

public void sendMessage(String channel, String message) {
if (!Strings.isNullOrEmpty(message)) {
try {
Message slackMessage = new Message();
if (Strings.isNullOrEmpty(channel)) {
throw new SlackNotificationSenderException(
"Channel cannot be empty for a Slack notification");
}
slackMessage.setChannel(channel);
Attachment attachment = new Attachment();
attachment.setText(message);
attachment.setColor(COLOR_WARNING);
attachment.getMrkdwnIn().add(MRKDWNIN_TEXT);
slackMessage.getAttachments().add(attachment);

this.slackClient.sendInstantMessage(slackMessage);
} catch (SlackClientException e) {
throw new SlackNotificationSenderException(
"Error sending Slack notification: " + e.getMessage(), e);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.box.l10n.mojito.cli.command.utils;

public class SlackNotificationSenderException extends RuntimeException {
public SlackNotificationSenderException(String message) {
super(message);
}

public SlackNotificationSenderException(String message, Throwable t) {
super(message, t);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import static org.mockito.Mockito.doNothing;

import com.beust.jcommander.Parameters;
import com.box.l10n.mojito.cli.command.extraction.ExtractionDiffNotificationSender;
import com.box.l10n.mojito.cli.command.utils.SlackNotificationSender;
import java.util.Optional;
import org.mockito.Mockito;
import org.springframework.context.annotation.Scope;
Expand All @@ -17,19 +17,19 @@
commandNames = {"extract-diff-test"},
commandDescription = "Class to test Slack notifications for the extract-diff command")
public class ExtractionDiffCommandForTest extends ExtractionDiffCommand {
private Optional<ExtractionDiffNotificationSender> mockedNotificationSender = empty();
private Optional<SlackNotificationSender> mockedNotificationSender = empty();

@Override
protected Optional<ExtractionDiffNotificationSender> getNotificationSender() {
Optional<ExtractionDiffNotificationSender> notificationSender = super.getNotificationSender();
protected Optional<SlackNotificationSender> getNotificationSender() {
Optional<SlackNotificationSender> notificationSender = super.getNotificationSender();
this.mockedNotificationSender = notificationSender.map(Mockito::spy);
this.mockedNotificationSender.ifPresent(
extractionDiffNotificationSender ->
doNothing().when(extractionDiffNotificationSender).sendMessage(anyString()));
slackNotificationSender ->
doNothing().when(slackNotificationSender).sendMessage(anyString(), anyString()));
return this.mockedNotificationSender;
}

public Optional<ExtractionDiffNotificationSender> getMockedNotificationSender() {
public Optional<SlackNotificationSender> getMockedNotificationSender() {
return this.mockedNotificationSender;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import static org.mockito.Mockito.verify;

import com.box.l10n.mojito.cli.CLITestBase;
import com.box.l10n.mojito.cli.command.extraction.ExtractionDiffNotificationSender;
import com.box.l10n.mojito.cli.command.utils.SlackNotificationSender;
import com.box.l10n.mojito.entity.Repository;
import com.box.l10n.mojito.service.tm.search.TextUnitDTO;
import com.box.l10n.mojito.service.tm.search.TextUnitSearcher;
Expand Down Expand Up @@ -549,11 +549,12 @@ public void testMaxStringsBlockFailure() throws Exception {

ExtractionDiffCommandForTest extractionDiffCommandForTest =
l10nJCommander.getCommand(ExtractionDiffCommandForTest.class);
Optional<ExtractionDiffNotificationSender> mockedNotificationSender =
Optional<SlackNotificationSender> mockedNotificationSender =
extractionDiffCommandForTest.getMockedNotificationSender();
assertTrue(mockedNotificationSender.isPresent());
verify(mockedNotificationSender.get(), times(1))
.sendMessage(
"CHANNEL_ID",
String.format(ExtractionDiffCommand.MAX_STRINGS_ADDED_BLOCK_MESSAGE, "branch", 3, 2));
Assert.assertEquals(1L, l10nJCommander.getExitCode());
Mockito.verify(l10nJCommander.consoleWriter, Mockito.times(1))
Expand Down Expand Up @@ -607,6 +608,7 @@ public void testMaxStringsBlockFailure() throws Exception {
assertTrue(mockedNotificationSender.isPresent());
verify(mockedNotificationSender.get(), times(1))
.sendMessage(
"CHANNEL_ID",
String.format(ExtractionDiffCommand.MAX_STRINGS_REMOVED_BLOCK_MESSAGE, "branch", 3, 2));
Assert.assertEquals(1L, l10nJCommander.getExitCode());
Mockito.verify(l10nJCommander.consoleWriter, Mockito.times(1))
Expand Down
Loading

0 comments on commit c1c7e22

Please sign in to comment.