diff --git a/app/forms/frontend_error_form.rb b/app/forms/frontend_error_form.rb new file mode 100644 index 00000000000..5c8e91929f8 --- /dev/null +++ b/app/forms/frontend_error_form.rb @@ -0,0 +1,30 @@ +class FrontendErrorForm + include ActiveModel::Model + include ActionView::Helpers::TranslationHelper + + validate :validate_filename_extension + validate :validate_filename_host + + attr_reader :filename + + def submit(filename:) + @filename = filename + + FormResponse.new(success: valid?, errors:, serialize_error_details_only: true) + end + + private + + def validate_filename_extension + return if File.extname(filename.to_s) == '.js' + errors.add(:filename, :invalid_extension, message: t('errors.general')) + end + + def validate_filename_host + begin + return if URI(filename.to_s).host == IdentityConfig.store.domain_name + rescue URI::InvalidURIError; end + + errors.add(:filename, :invalid_host, message: t('errors.general')) + end +end diff --git a/app/services/frontend_error_logger.rb b/app/services/frontend_error_logger.rb index fd5a08ae8c5..d292ab2a57a 100644 --- a/app/services/frontend_error_logger.rb +++ b/app/services/frontend_error_logger.rb @@ -2,6 +2,8 @@ class FrontendErrorLogger class FrontendError < StandardError; end def self.track_error(name:, message:, stack:, filename:) + return unless FrontendErrorForm.new.submit(filename:).success? + NewRelic::Agent.notice_error( FrontendError.new, expected: true, diff --git a/spec/controllers/frontend_log_controller_spec.rb b/spec/controllers/frontend_log_controller_spec.rb index 9ca398cedbb..35f4aed8324 100644 --- a/spec/controllers/frontend_log_controller_spec.rb +++ b/spec/controllers/frontend_log_controller_spec.rb @@ -206,6 +206,8 @@ end it 'notices the error to NewRelic instead of analytics logger' do + allow_any_instance_of(FrontendErrorForm).to receive(:submit). + and_return(FormResponse.new(success: true)) expect(fake_analytics).not_to receive(:track_event) expect(NewRelic::Agent).to receive(:notice_error).with( FrontendErrorLogger::FrontendError.new, diff --git a/spec/forms/frontend_error_form_spec.rb b/spec/forms/frontend_error_form_spec.rb new file mode 100644 index 00000000000..d800d1fe0df --- /dev/null +++ b/spec/forms/frontend_error_form_spec.rb @@ -0,0 +1,69 @@ +require 'rails_helper' + +RSpec.describe FrontendErrorForm do + let(:filename) { 'https://example.com/foo.js' } + + subject(:form) { described_class.new } + + before do + allow(IdentityConfig.store).to receive(:domain_name).and_return('example.com') + end + + describe '#submit' do + subject(:result) { form.submit(filename:) } + + context 'with valid filename' do + let(:filename) { 'https://example.com/foo.js' } + + it 'is successful' do + expect(result.success?).to eq(true) + expect(result.errors).to eq({}) + end + end + + context 'without filename' do + let(:filename) { nil } + + it 'is unsuccessful' do + expect(result.success?).to eq(false) + expect(result.errors).to eq(filename: [t('errors.general'), t('errors.general')]) + end + end + + context 'with filename without extension' do + let(:filename) { 'https://example.com/foo' } + + it 'is unsuccessful' do + expect(result.success?).to eq(false) + expect(result.errors).to eq(filename: [t('errors.general')]) + end + end + + context 'with filename having extension other than js' do + let(:filename) { 'https://example.com/foo.txt' } + + it 'is unsuccessful' do + expect(result.success?).to eq(false) + expect(result.errors).to eq(filename: [t('errors.general')]) + end + end + + context 'with filename from a different host' do + let(:filename) { 'https://wrong.example.com/foo.js' } + + it 'is unsuccessful' do + expect(result.success?).to eq(false) + expect(result.errors).to eq(filename: [t('errors.general')]) + end + end + + context 'with filename that cannot be parsed as url' do + let(:filename) { '{' } + + it 'is unsuccessful' do + expect(result.success?).to eq(false) + expect(result.errors).to eq(filename: [t('errors.general'), t('errors.general')]) + end + end + end +end diff --git a/spec/services/frontend_error_logger_spec.rb b/spec/services/frontend_error_logger_spec.rb index 33c570b5ce3..7f1922e35eb 100644 --- a/spec/services/frontend_error_logger_spec.rb +++ b/spec/services/frontend_error_logger_spec.rb @@ -1,6 +1,13 @@ require 'rails_helper' RSpec.describe FrontendErrorLogger do + let(:valid) { true } + + before do + allow_any_instance_of(FrontendErrorForm).to receive(:submit). + and_return(FormResponse.new(success: valid)) + end + describe '.track_event' do it 'notices an expected error to NewRelic with custom parameters' do expect(NewRelic::Agent).to receive(:notice_error).with( @@ -11,7 +18,7 @@ name: 'name', message: 'message', stack: 'stack', - filename: 'filename', + filename: 'filename.js', }, }, ) @@ -20,8 +27,23 @@ name: 'name', message: 'message', stack: 'stack', - filename: 'filename', + filename: 'filename.js', ) end + + context 'with unsuccessful validation of request parameters' do + let(:valid) { false } + + it 'does not notice an error' do + expect(NewRelic::Agent).not_to receive(:notice_error) + + FrontendErrorLogger.track_error( + name: 'name', + message: 'message', + stack: 'stack', + filename: 'filename.js', + ) + end + end end end