[rush] Design Proposal: Rush Custom Tips

theJiawen commented 1 year ago

Summary

Currently if Rush encounters an error such as detecting inconsistent versions in the common workspace, it displays a generic error message. While generic error messages can be helpful at times, they may not always provide sufficient information. Hence we want to add a feature to rush to be able to display customized error messages, For instance, these messages could include links to documentation that explains the problem and its solution.

To achieve this goal, we propose introducing a new feature called "Rush Custom Tips" (alternative names are welcome). Users will be able to create a new configuration file named rush-custom-tips.json, which will allow them to append tip after the original message.

Demo in one screenshot

Prototype PR: #4208

In the below screenshot we are running "rush update" in VSCode. We created a new config file rush-custom-tips.json and saying that if rush encounters PNPM_MISMATCH_DEPENDENCY error, display a custom tip after the original message. The screenshot shows the code change, and how to reproduce the output.

Some design decisions

1. Why append the tip but not replacing the original message?

There are two reasons.

For one, it is unnecessary and even harmful to replace the original message. The replaced error message may not explain the error well, making it difficult for developers to understand (and even harder to find the root cause). Additionally, the original error message provides more context.

Secondly, it's harder to replace the original message because there are a lot of messages are printed by child process (e.g., "rush update" spawns "pnpm install"). Allowing user to replace the message means that we would have to interpret what's printed by child_process (instead of letting the child process print itself), which is a lot of work to do every time we want to expose more customizable tips.

2. Why not use regular expression to match the message?

I also thought that might be more flexible but after discussing with @octogonz, we reached a consensus that pattern-matching is not ideal. Pete:

I suspect people will be concerned that pattern-matching console.log() may be too brittle. For example, suppose a company has implemented custom error messages in their monorepo. But then we change Rush so that the error message is worded differently. Now the custom error messages will no longer match the pattern. How would anybody detect this regression? You would only find out if (1) the error DID happen and (2) someone noticed that the custom message is no longer being printed.

However, in the future we could design other flexible ways to expose new ids (e.g., allowing the Plugin to expose it's own tip ids).

theJiawen commented 1 year ago

cc @octogonz

octogonz commented 1 year ago

Some design feedback:

The config file rush-custom-tips.json should probably go in common/config/rush/custom-tips.json with other Rush config files
Generally I think the APIs can be named ICustomTipItemJson instead of IRushCustomTipItemJson. The name "custom tips" is fairly unique, so there shouldn't be confusion about whether it applies to rush or not. (Whereas for example RushConfiguration did get this prefix, because there are many possible configurations.)

octogonz commented 1 year ago

After further discussion with @theJiawen, this was the updated config file that we ended up with:

common/config/rush/custom-tips.json

/**
 * This configuration file allows repo maintainers to configure extra details to be
 * printed alongside certain Rush messages.  More documentation is available on the
 * Rush website: https://rushjs.io
 */
{
  "$schema": "https://developer.microsoft.com/json-schemas/rush/v5/custom-tips.schema.json",

  /**
   * If specified, this prefix will be prepended to any the tip messages when they are displayed.
   * The default value is an empty string.
   */
  // "defaultMessagePrefix": "⭐️ Contoso monorepo tip: ",

  /**
   * Specifies the custom tips to be displayed by Rush.
   */
  "customTips": [
    {
      /**
       * (REQUIRED) An identifier indicating a message that may be printed by Rush.
       * If that message is printed, then this custom tip will be shown.
       * Consult the Rush documentation for the current list of possible identifiers.
       */
      "tipId": "TIP_RUSH_INCONSISTENT_VERSIONS",

       /**
        * (REQUIRED) The message text to be displayed for this tip.
        */
       "message": "For additional troubleshooting information, refer this wiki article:\n\nhttps://intranet.contoso.com/docs/pnpm-mismatch",

       /**
        * Overrides the "defaultMessagePrefix" for this tip.
        * Specify an empty string to omit the "defaultMessagePrefix" entirely.
        */
       // "messagePrefix": ""
    }
  ]
}

octogonz commented 1 year ago

PR #4208 originally included a severity field, allowing custom-tips.json to specify tips as error, warning, or info. However after further discussion, we determined that in most cases the tip's severity should be the same as the underlying Rush message. And if not, then it should certainly not be more severe, since custom-tips.json probably does not have enough feature context to make such determinations.

We are thinking that visibility or importance is probably more relevant for tips than severity.

Compare these examples:

tipVisibility=normal


// Rush message
terminal.writeWarningLine("Warning: Your PNPM version is not supported");

// Custom tip message terminal.writeLine("For info about upgrading PNPM, see our wiki: http://...");


2. **tipVisibility=important**
```ts
// Rush message
terminal.writeWarningLine("Warning: Your PNPM version is not supported");

// Custom tip message (BOLD WHITE COLOR)
terminal.writeLine("+===============================================================+");
terminal.writeLine("| Important: Using outdated software is against company policy. |");
terminal.writeLine("| See the wiki for details: http://...                          |");
terminal.writeLine("+===============================================================+");

tipVisibilty=hint


// Rush message
terminal.writeWarningLine("Warning: Your PNPM version is not supported");

// Custom tip message (DIM CYAN COLOR) terminal.writeLine("This is a legacy repo. You can safely ignore outdated version warnings by using --bypass-policy.");



The second case is calling more attention to the problem. But it should probably not be upgrading Rush's process exit code.  (That kind of change really belongs in the underlying feature's configuration, I think.)

The third example is dismissing an error.

This feature idea seems valuable, but we decided to tackle it in a separate PR that is maybe focused on output formatting.

octogonz commented 1 year ago

I also think we should aim for visual presentation that helps end users to clearly understand that custom tips are special messages for their repo, not standard output of Rush. Here is an example from testing PR https://github.com/microsoft/rushstack/pull/4208:

The Found 2 mis-matching dependencies message is from Rush, but For additional troubleshooting is coming from custom-tips.json. Distinguishing them has a few benefits:

End users may be MORE interested in the tips, because they are messages from their own team.
In some cases, end users might disregard tips, understanding that they may not be as authoritative as official Rush messaging.
When investigating problem reports, we don't want the Rush maintainers to get confused by output that turned out to be a custom tip

microsoft / rushstack