From e20c30987c6a21c3b43e8cc281b36a66c0a31ccf Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Fri, 24 May 2019 17:25:55 +0300 Subject: [PATCH] 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 --- .../discourse/lib/utilities.js.es6 | 34 ++++++++++++++++++ .../stylesheets/common/base/discourse.scss | 10 ++++++ app/models/theme.rb | 8 +++++ app/models/theme_field.rb | 6 ++-- config/locales/client.en.yml | 1 + lib/theme_javascript_compiler.rb | 9 ++++- spec/models/theme_spec.rb | 35 +++++++++++++++++-- 7 files changed, 97 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6 index 4d5240b66f1..c022bfd5c98 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: `${path}` + }); + 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 76111cc9d36..36d24fe5b7a 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 74b104946d2..7daeb4b9393 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 447a3467215..cceb9493de2 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 3064a8fd154..4b2c6b34eea 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 4bbdec5fa4f..dc1dbabd7da 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 2a6767d4275..f1f1db49575 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("I am bold") end + it "changing theme name should re-transpile HTML theme fields" do + theme.update!(name: "old_name") + html = <<~HTML + + 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 = < @@ -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: '') 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\\\""; 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); + } }); } })();