FEATURE: detect theme errors and catch them (#7589)

FEATURE: detect theme errors and catch them (#7589)

  • FEATURE: detect theme errors and catch them

  • Bump COMPILER_VERSION

  • Feedback

  • Override eslint no console for one line

  • Can’t use our ajax method

  • remove emoji from translation file

diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6
index 4d5240b..c022bfd 100644
--- a/app/assets/javascripts/discourse/lib/utilities.js.es6
+++ b/app/assets/javascripts/discourse/lib/utilities.js.es6
@@ -660,5 +660,39 @@ export function postRNWebviewMessage(prop, value) {
   }
 }
 
+function reportToLogster(name, error) {
+  const data = {
+    message: `${name} theme/component is throwing errors`,
+    stacktrace: error.stack
+  };
+
+  Ember.$.ajax(`${Discourse.BaseUri}/logs/report_js_error`, {
+    data,
+    type: "POST",
+    cache: false
+  });
+}
+// this function is used in lib/theme_javascript_compiler.rb
+export function rescueThemeError(name, error, api) {
+  /* eslint-disable-next-line no-console */
+  console.error(`"${name}" error:`, error);
+  reportToLogster(name, error);
+
+  const currentUser = api.getCurrentUser();
+  if (!currentUser || !currentUser.admin) {
+    return;
+  }
+
+  const path = `${Discourse.BaseUri}/admin/customize/themes`;
+  const message = I18n.t("themes.broken_theme_alert", {
+    theme: name,
+    path: `<a href="${path}">${path}</a>`
+  });
+  const alertDiv = document.createElement("div");
+  alertDiv.classList.add("broken-theme-alert");
+  alertDiv.innerHTML = `⚠️ ${message}`;
+  document.body.prepend(alertDiv);
+}
+
 // This prevents a mini racer crash
 export default {};
diff --git a/app/assets/stylesheets/common/base/discourse.scss b/app/assets/stylesheets/common/base/discourse.scss
index 76111cc..36d24fe 100644
--- a/app/assets/stylesheets/common/base/discourse.scss
+++ b/app/assets/stylesheets/common/base/discourse.scss
@@ -732,3 +732,13 @@ table {
     color: $danger;
   }
 }
+
+.broken-theme-alert {
+  font-size: $base-font-size;
+  font-weight: bold;
+  padding: 5px 0;
+  background: $danger-medium;
+  text-align: center;
+  z-index: z("max");
+  color: $secondary;
+}
diff --git a/app/models/theme.rb b/app/models/theme.rb
index 74b1049..7daeb4b 100644
--- a/app/models/theme.rb
+++ b/app/models/theme.rb
@@ -53,11 +53,19 @@ class Theme < ActiveRecord::Base
 
     Theme.expire_site_cache! if saved_change_to_user_selectable? || saved_change_to_name?
     notify_with_scheme = saved_change_to_color_scheme_id?
+    name_changed = saved_change_to_name?
 
     reload
     settings_field&.ensure_baked! # Other fields require setting to be **baked**
     theme_fields.each(&:ensure_baked!)
 
+    if name_changed
+      theme_fields.select { |f| f.basic_html_field? }.each do |f|
+        f.value_baked = nil
+        f.ensure_baked!
+      end
+    end
+
     remove_from_cache!
     clear_cached_settings!
     ColorScheme.hex_cache.clear
diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index 447a346..cceb949 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -63,7 +63,7 @@ class ThemeField < ActiveRecord::Base
   validates :name, format: { with: /\A[a-z_][a-z0-9_-]*\z/i },
                    if: Proc.new { |field| ThemeField.theme_var_type_ids.include?(field.type_id) }
 
-  COMPILER_VERSION = 10
+  COMPILER_VERSION = 11
 
   belongs_to :theme
 
@@ -71,7 +71,7 @@ class ThemeField < ActiveRecord::Base
     errors = []
     javascript_cache || build_javascript_cache
 
-    js_compiler = ThemeJavascriptCompiler.new(theme_id)
+    js_compiler = ThemeJavascriptCompiler.new(theme_id, self.theme.name)
 
     doc = Nokogiri::HTML.fragment(html)
 
@@ -153,7 +153,7 @@ class ThemeField < ActiveRecord::Base
   def process_translation
     errors = []
     javascript_cache || build_javascript_cache
-    js_compiler = ThemeJavascriptCompiler.new(theme_id)
+    js_compiler = ThemeJavascriptCompiler.new(theme_id, self.theme.name)
     begin
       data = translation_data
 
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 3064a8f..4b2c6b3 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -187,6 +187,7 @@ en:
 
     themes:
       default_description: "Default"
+      broken_theme_alert: "Theme/component %{theme} has errors which may prevent your site from working correctly! You can disable it at %{path}."
 
     s3:
       regions:
diff --git a/lib/theme_javascript_compiler.rb b/lib/theme_javascript_compiler.rb
index 4bbdec5..dc1dbab 100644
--- a/lib/theme_javascript_compiler.rb
+++ b/lib/theme_javascript_compiler.rb
@@ -147,9 +147,10 @@ class ThemeJavascriptCompiler
 
   attr_accessor :content
 
-  def initialize(theme_id)
+  def initialize(theme_id, theme_name)
     @theme_id = theme_id
     @content = +""
+    @theme_name = theme_name
   end
 
   def prepend_settings(settings_hash)
@@ -212,12 +213,18 @@ class ThemeJavascriptCompiler
     wrapped = <<~PLUGIN_API_JS
       (function() {
         if ('Discourse' in window && typeof Discourse._registerPluginCode === 'function') {
+          const __theme_name__ = "#{@theme_name.gsub('"', "\\\"")}";
           const settings = Discourse.__container__
             .lookup("service:theme-settings")
             .getObjectForTheme(#{@theme_id});
           const themePrefix = (key) => `theme_translations.#{@theme_id}.${key}`;
           Discourse._registerPluginCode('#{version}', api => {
+            try {
             #{es6_source}
+            } catch(err) {
+              const rescue = require("discourse/lib/utilities").rescueThemeError;
+              rescue(__theme_name__, err, api);
+            }
           });
         }
       })();
diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb
index 2a6767d..f1f1db4 100644
--- a/spec/models/theme_spec.rb
+++ b/spec/models/theme_spec.rb
@@ -109,6 +109,24 @@ describe Theme do
     expect(Theme.lookup_field(theme.id, :desktop, "head_tag")).to eq("<b>I am bold</b>")
   end
 
+  it "changing theme name should re-transpile HTML theme fields" do
+    theme.update!(name: "old_name")
+    html = <<~HTML
+      <script type='text/discourse-plugin' version='0.1'>
+        const x = 1;
+      </script>
+    HTML
+    theme.set_field(target: :common, name: "head_tag", value: html)
+    theme.save!
+    field = theme.theme_fields.where(value: html).first
+    old_value = field.value_baked
+
+    theme.update!(name: "new_name")
+    field.reload
+    new_value = field.value_baked
+    expect(old_value).not_to eq(new_value)
+  end
+
   it 'should precompile fragments in body and head tags' do
     with_template = <<HTML
     <script type='text/x-handlebars' name='template'>
@@ -329,6 +347,7 @@ HTML
     end
 
     it "allows values to be used in JS" do
+      theme.name = 'awesome theme"'
       theme.set_field(target: :settings, name: :yaml, value: "name: bob")
       theme_field = theme.set_field(target: :common, name: :after_header, value: '<script type="text/discourse-plugin" version="1.0">alert(settings.name); let a = ()=>{};</script>')
       theme.save!
@@ -343,12 +362,18 @@ HTML
       })();
       (function () {
         if ('Discourse' in window && typeof Discourse._registerPluginCode === 'function') {
+          var __theme_name__ = "awesome theme\\\"";
           var settings = Discourse.__container__.lookup("service:theme-settings").getObjectForTheme(#{theme.id});
           var themePrefix = function themePrefix(key) {
             return 'theme_translations.#{theme.id}.' + key;
           };
           Discourse._registerPluginCode('1.0', function (api) {
-            alert(settings.name);var a = function a() {};
+            try {
+              alert(settings.name);var a = function a() {};
+            } catch (err) {
+              var rescue = require("discourse/lib/utilities").rescueThemeError;
+              rescue(__theme_name__, err, api);
+            }
           });
         }
       })();
@@ -372,12 +397,18 @@ HTML
       })();
       (function () {
         if ('Discourse' in window && typeof Discourse._registerPluginCode === 'function') {
+          var __theme_name__ = "awesome theme\\\"";

[... diff too long, it was truncated ...]

GitHub sha: e20c3098

2 Likes