Skip to content

Conversation

@icanswiftabit
Copy link

@icanswiftabit icanswiftabit commented Jun 26, 2020

Change CommandError and MessageInfo to public.

TL:DR;

The motivation behind it was the desire to react to such error with a beautiful message.

Motivation

In our tool, we use SwiftNIO and ArgumentParser for handling slack commands. At one point a command can fail. For example when a user provides not expect a flag. This will produce CommandError. NIO's mapIfError will bring it to Error type. Hence at this moment, we can only print such error.

CommandError(commandStack: [SlackCommands.PR], parserError: ArgumentParser.ParserError.unknownOption(ArgumentParser.InputOrigin.Element.argumentIndex(0), ArgumentParser.Name.long("some-wrong-argument")))

Not very user friendly :(
But if CommandError and MessageInfo would be public. We could detect if the mapped error is a CommandError and use MessageInfo to print a much nicer message. Such as

Error: Missing expected argument '<project-name>'
Usage: Stats project <project-name> [--repository <repository>]
  See 'Stats project --help' for more information.

This is how we would use it

func handleWebHook(payload: SlackHookPayload) throws -> Future<String> {
    return commandDispatcher.dispatch(payload: payload).mapIfError { error -> String in
        guard let commandError = error as? ArgumentParser.CommandError, let commandType = commandError.commandStack.first.self else {
            return "Command failed with a message: \(error)"
        }
        return MessageInfo(error: commandError, type: commandType).fullText
    }
}

Checklist

  • I've added at least one test that validates that my change is working, if appropriate // Not applicable
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary // Not applicable

@icanswiftabit icanswiftabit changed the title Change ComandError and MessageInfo to public Change CommandError and MessageInfo to public Jun 26, 2020
@natecook1000
Copy link
Member

You can currently call message(for: error) or fullMessage(for: error) on your command types to convert an error thrown from ArgumentParser into a human-readable string. We aren't ready to expose CommandError or MessageInfo as public API yet — would you mind opening an issue with your use case, instead?

@icanswiftabit
Copy link
Author

Ok, these methods do help in our case. I can easily recreate command from SlackHookPayload. But I'm sure there is a case where knowing about CommandError would be great.
Ahh, a contribution was so close. Thanks for the help :) Here is the issue #199

@icanswiftabit icanswiftabit deleted the feature/public-command-error branch June 29, 2020 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants