FIX: silent failure when watched words contains invalid regex (#36132)

ZogStriP · web-flow · commit f07137e4c6f4 · 2025-11-25T21:56:33.000+01:00
If a watched words list contains any invalid regex, the test modal won't work because the "compiled" regex (that contains all the watched words) will be invalid. This PR introduces a fallback to matching individual watched words when the compiled regex is invalid as well as displaying the error for each invalid regex. <img width="1549" height="1359" alt="Screenshot 2025-11-20 at 10 34 00" src="https://github.com/user-attachments/assets/ed781d12-bc78-4d8c-98ac-bd49448a9718" /> https://github.com/user-attachments/assets/f9200ca9-eda7-4592-8a8e-437ab4a13953 Internal ref - t/168206
diff --git a/app/assets/stylesheets/admin/staff_logs.scss b/app/assets/stylesheets/admin/staff_logs.scss
@@ -307,6 +307,13 @@ table.screened-ip-addresses {
 }
 
 // Watched words
+.watched-word-regex-errors {
+  max-height: 200px;
+  overflow-y: auto;
+  list-style-type: disc;
+  padding-left: 20px;
+}
+
 .watched-word-box {
   display: inline-block;
   width: 250px;
diff --git a/app/services/word_watcher.rb b/app/services/word_watcher.rb
@@ -79,8 +79,12 @@ def self.compiled_regexps_for_action(action, engine: :ruby, raise_errors: false)
               r = word_to_regexp(word, match_word: SiteSetting.watched_words_regular_expressions?)
               begin
                 r if Regexp.new(r)
-              rescue RegexpError
+              rescue RegexpError => e
                 raise if raise_errors
+                Rails.logger.warn(
+                  "Watched word '#{word}' has invalid regex '#{r}' for #{action}: #{e.message}",
+                )
+                nil
               end
             end
             .select { |r| r.present? }
@@ -96,7 +100,15 @@ def self.compiled_regexps_for_action(action, engine: :ruby, raise_errors: false)
           ) if !SiteSetting.watched_words_regular_expressions?
 
         # Add case insensitive flag if needed
-        Regexp.new(regexp, group_key == :case_sensitive ? nil : Regexp::IGNORECASE)
+        begin
+          Regexp.new(regexp, group_key == :case_sensitive ? nil : Regexp::IGNORECASE)
+        rescue RegexpError => e
+          raise if raise_errors
+          Rails.logger.warn(
+            "Watched word compilation failed for #{action} (#{group_key}): #{e.message}. Regexp: #{regexp}",
+          )
+          nil
+        end
       end
       .compact
   end
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
@@ -7750,6 +7750,7 @@ en:
         clear_all: Clear all
         clear_all_confirm: "Are you sure you want to clear all watched words for the %{action} action?"
         invalid_regex: 'The watched word "%{word}" is an invalid regular expression.'
+        invalid_regex_multiple: "Invalid Regular Expressions:"
         regex_warning: '<a href="%{basePath}/admin/site_settings/category/all_results?filter=watched%20words%20regular%20expressions%20">Watched words are regular expressions</a> and they do not automatically include word boundaries. If you want the regular expression to match whole words, include <code>\b</code> at the start and end of your regular expression.'
         actions:
           block: "Block"
diff --git a/frontend/discourse/admin/components/modal/watched-word-testing.gjs b/frontend/discourse/admin/components/modal/watched-word-testing.gjs
@@ -1,5 +1,5 @@
 import Component from "@glimmer/component";
-import { tracked } from "@glimmer/tracking";
+import { cached, tracked } from "@glimmer/tracking";
 import { Textarea } from "@ember/component";
 import DModal from "discourse/components/d-modal";
 import { or } from "discourse/truth-helpers";
@@ -20,70 +20,126 @@ export default class WatchedWordTesting extends Component {
     return this.args.model.watchedWord.nameKey === "link";
   }
 
-  get matches() {
-    if (
-      !this.value ||
-      this.args.model.watchedWord.compiledRegularExpression.length === 0
-    ) {
-      return [];
+  cleanErrorMessage(message) {
+    const parts = message.split(": ");
+    return parts[parts.length - 1];
+  }
+
+  @cached
+  get matchesAndErrors() {
+    const errors = {};
+
+    const addError = (word, message) => {
+      errors[word] ??= this.cleanErrorMessage(message);
+    };
+
+    const errorsToArray = () =>
+      Object.entries(errors).map(([word, error]) => ({ word, error }));
+
+    if (!this.value) {
+      return { matches: [], errors: [] };
     }
 
     if (this.isReplace || this.isLink) {
       const matches = [];
       this.args.model.watchedWord.words.forEach((word) => {
-        const regexp = new RegExp(
-          word.regexp,
-          word.case_sensitive ? "gu" : "gui"
-        );
-        let match;
-
-        while ((match = regexp.exec(this.value)) !== null) {
-          matches.push({
-            match: match[1],
-            replacement: word.replacement,
-          });
+        try {
+          const regexp = new RegExp(
+            word.regexp,
+            word.case_sensitive ? "gu" : "gui"
+          );
+          let match;
+
+          while ((match = regexp.exec(this.value)) !== null) {
+            matches.push({
+              match: match[1],
+              replacement: word.replacement,
+            });
+          }
+        } catch (e) {
+          addError(word.word, e.message);
         }
       });
-      return matches;
+      return { matches, errors: errorsToArray() };
     }
 
     if (this.isTag) {
       const matches = new Map();
       this.args.model.watchedWord.words.forEach((word) => {
-        const regexp = new RegExp(
-          word.regexp,
-          word.case_sensitive ? "gu" : "gui"
-        );
-        let match;
-
-        while ((match = regexp.exec(this.value)) !== null) {
-          if (!matches.has(match[1])) {
-            matches.set(match[1], new Set());
+        try {
+          const regexp = new RegExp(
+            word.regexp,
+            word.case_sensitive ? "gu" : "gui"
+          );
+          let match;
+
+          while ((match = regexp.exec(this.value)) !== null) {
+            if (!matches.has(match[1])) {
+              matches.set(match[1], new Set());
+            }
+
+            const tags = matches.get(match[1]);
+            word.replacement.split(",").forEach((tag) => tags.add(tag));
           }
-
-          const tags = matches.get(match[1]);
-          word.replacement.split(",").forEach((tag) => tags.add(tag));
+        } catch (e) {
+          addError(word.word, e.message);
         }
       });
 
-      return Array.from(matches, ([match, tagsSet]) => ({
-        match,
-        tags: Array.from(tagsSet),
-      }));
+      return {
+        matches: Array.from(matches, ([match, tagsSet]) => ({
+          match,
+          tags: Array.from(tagsSet),
+        })),
+        errors: errorsToArray(),
+      };
     }
 
     let matches = [];
+    let hasCompiledExpressionError = false;
+
     this.args.model.watchedWord.compiledRegularExpression.forEach((entry) => {
-      const [regexp, options] = Object.entries(entry)[0];
-      const wordRegexp = new RegExp(
-        regexp,
-        options.case_sensitive ? "gu" : "gui"
-      );
+      try {
+        const [regexp, options] = Object.entries(entry)[0];
+        const wordRegexp = new RegExp(
+          regexp,
+          options.case_sensitive ? "gu" : "gui"
+        );
 
-      matches.push(...(this.value.match(wordRegexp) || []));
+        matches.push(...(this.value.match(wordRegexp) || []));
+      } catch {
+        hasCompiledExpressionError = true;
+      }
     });
 
-    return matches;
+    if (hasCompiledExpressionError) {
+      matches = [];
+      this.args.model.watchedWord.words.forEach((word) => {
+        try {
+          const regexp = new RegExp(
+            word.regexp,
+            word.case_sensitive ? "gu" : "gui"
+          );
+          let match;
+
+          while ((match = regexp.exec(this.value)) !== null) {
+            matches.push(match[1] || match[0]);
+          }
+        } catch (e) {
+          addError(word.word, e.message);
+        }
+      });
+    }
+
+    return { matches, errors: errorsToArray() };
+  }
+
+  get matches() {
+    return this.matchesAndErrors.matches;
+  }
+
+  get regexErrors() {
+    return this.matchesAndErrors.errors;
   }
 
   <template>
diff --git a/frontend/discourse/admin/controllers/admin-watched-words/action.js b/frontend/discourse/admin/controllers/admin-watched-words/action.js
@@ -1,4 +1,4 @@
-import { tracked } from "@glimmer/tracking";
+import { cached, tracked } from "@glimmer/tracking";
 import Controller, { inject as controller } from "@ember/controller";
 import { action } from "@ember/object";
 import { or } from "@ember/object/computed";
@@ -34,14 +34,35 @@ export default class AdminWatchedWordsActionController extends Controller {
     );
   }
 
-  get regexpError() {
-    for (const { regexp, word } of this.currentAction.words) {
+  @cached
+  get regexpErrors() {
+    const errors = [];
+    const seen = new Set();
+
+    if (!this.currentAction?.words) {
+      return errors;
+    }
+
+    for (const { regexp, word, id } of this.currentAction.words) {
+      if (!regexp) {
+        continue;
+      }
+
       try {
-        RegExp(regexp);
-      } catch {
-        return i18n("admin.watched_words.invalid_regex", { word });
+        // eslint-disable-next-line no-new
+        new RegExp(regexp, "u");
+      } catch (e) {
+        const key = `${id}:${word}:${e.message}`;
+        if (!seen.has(key)) {
+          seen.add(key);
+          const parts = e.message.split(": ");
+          const cleanError = parts[parts.length - 1];
+          errors.push({ word, error: cleanError });
+        }
       }
     }
+
+    return errors;
   }
 
   get actionDescription() {
diff --git a/frontend/discourse/admin/templates/admin-watched-words/action.gjs b/frontend/discourse/admin/templates/admin-watched-words/action.gjs
@@ -8,8 +8,18 @@ import basePath from "discourse/helpers/base-path";
 import { i18n } from "discourse-i18n";
 
 export default <template>
-  {{#if @controller.regexpError}}
-    <div class="alert alert-error">{{@controller.regexpError}}</div>
+  {{#if @controller.regexpErrors.length}}
+    <div class="alert alert-error">
+      <strong>{{i18n "admin.watched_words.invalid_regex_multiple"}}</strong>
+      <ul class="watched-word-regex-errors">
+        {{#each @controller.regexpErrors as |error|}}
+          <li>
+            <strong>{{error.word}}</strong>:
+            {{error.error}}
+          </li>
+        {{/each}}
+      </ul>
+    </div>
   {{/if}}
 
   <div class="watched-word-controls">
diff --git a/frontend/discourse/tests/acceptance/admin-watched-words-test.js b/frontend/discourse/tests/acceptance/admin-watched-words-test.js
diff --git a/spec/services/word_watcher_spec.rb b/spec/services/word_watcher_spec.rb
