diff --git a/app/forms/frontend_error_form.rb b/app/forms/frontend_error_form.rb index e7fcd196efc..f9ed8046632 100644 --- a/app/forms/frontend_error_form.rb +++ b/app/forms/frontend_error_form.rb @@ -7,10 +7,11 @@ class FrontendErrorForm validate :validate_filename_extension validate :validate_filename_host - attr_reader :filename + attr_reader :filename, :error_id - def submit(filename:) + def submit(filename:, error_id:) @filename = filename + @error_id = error_id FormResponse.new(success: valid?, errors:, serialize_error_details_only: true) end @@ -18,11 +19,13 @@ def submit(filename:) private def validate_filename_extension - return if File.extname(filename.to_s) == '.js' + return if error_id || File.extname(filename.to_s) == '.js' errors.add(:filename, :invalid_extension, message: t('errors.general')) end def validate_filename_host + return if error_id + begin return if URI(filename.to_s).host == IdentityConfig.store.domain_name rescue URI::InvalidURIError; end diff --git a/app/javascript/packages/analytics/README.md b/app/javascript/packages/analytics/README.md index 752f2f17633..e33fb2b669a 100644 --- a/app/javascript/packages/analytics/README.md +++ b/app/javascript/packages/analytics/README.md @@ -8,6 +8,9 @@ Utilities and custom elements for logging events and errors in the application. Track an event or error from your code using exported function members. +Since JavaScript may be bundled and minified in production environments, including an `errorId` is +required to uniquely identify the source of the error. + ```ts import { trackEvent, trackError } from '@18f/identity-analytics'; @@ -18,7 +21,7 @@ button.addEventListener('click', () => { try { doSomethingRisky(); } catch (error) { - trackError(error); + trackError(error, { errorId: 'exampleId' }); } ``` diff --git a/app/javascript/packages/analytics/index.spec.ts b/app/javascript/packages/analytics/index.spec.ts index 6a3a374c125..5253d73ed68 100644 --- a/app/javascript/packages/analytics/index.spec.ts +++ b/app/javascript/packages/analytics/index.spec.ts @@ -93,7 +93,7 @@ describe('trackError', () => { }); it('tracks event', async () => { - trackError(new Error('Oops!')); + trackError(new Error('Oops!'), { errorId: 'exampleId' }); expect(global.navigator.sendBeacon).to.have.been.calledOnce(); @@ -101,12 +101,13 @@ describe('trackError', () => { expect(actualEndpoint).to.eql(endpoint); const { event, payload } = JSON.parse(await data.text()); - const { name, message, stack } = payload; + const { name, message, stack, error_id: errorId } = payload; expect(event).to.equal('Frontend Error'); expect(name).to.equal('Error'); expect(message).to.equal('Oops!'); expect(stack).to.be.a('string'); + expect(errorId).to.equal('exampleId'); }); context('with event parameter', () => { diff --git a/app/javascript/packages/analytics/index.ts b/app/javascript/packages/analytics/index.ts index 22fa2d91308..45e546195a4 100644 --- a/app/javascript/packages/analytics/index.ts +++ b/app/javascript/packages/analytics/index.ts @@ -2,6 +2,11 @@ import { getConfigValue } from '@18f/identity-config'; export { default as isTrackableErrorEvent } from './is-trackable-error-event'; +/** + * Metadata used to identify the source of an error. + */ +type ErrorMetadata = { errorId?: never; filename: string } | { errorId: string; filename?: never }; + /** * Logs an event. * @@ -24,8 +29,8 @@ export function trackEvent(event: string, payload?: object) { * Logs an error. * * @param error Error object. - * @param event Error event, if error is caught using an `error` event handler. Including this can - * add additional resolution to the logged error, notably the filename where the error occurred. + * @param metadata Metadata used to identify the source of an error, including either the filename + * from an ErrorEvent object, or a unique identifier. */ -export const trackError = ({ name, message, stack }: Error, event?: ErrorEvent) => - trackEvent('Frontend Error', { name, message, stack, filename: event?.filename }); +export const trackError = ({ name, message, stack }: Error, { filename, errorId }: ErrorMetadata) => + trackEvent('Frontend Error', { name, message, stack, filename, error_id: errorId }); diff --git a/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts b/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts index 45f6e33afeb..6177b63732a 100644 --- a/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts +++ b/app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts @@ -53,7 +53,7 @@ class CaptchaSubmitButtonElement extends HTMLElement { try { token = await this.recaptchaClient!.execute(siteKey!, { action }); } catch (error) { - trackError(error); + trackError(error, { errorId: 'recaptchaExecute' }); } this.tokenInput.value = token; diff --git a/app/javascript/packages/webauthn/webauthn-verify-button-element.ts b/app/javascript/packages/webauthn/webauthn-verify-button-element.ts index bafbdb6bd69..9acca615c1a 100644 --- a/app/javascript/packages/webauthn/webauthn-verify-button-element.ts +++ b/app/javascript/packages/webauthn/webauthn-verify-button-element.ts @@ -54,7 +54,7 @@ class WebauthnVerifyButtonElement extends HTMLElement { this.setInputValue('signature', result.signature); } catch (error) { if (!isExpectedWebauthnError(error, { isVerifying: true })) { - trackError(error); + trackError(error, { errorId: 'webauthnVerify' }); } if (isUserVerificationScreenLockError(error)) { diff --git a/app/javascript/packs/track-errors.ts b/app/javascript/packs/track-errors.ts index fb4582900ad..c0686e2ec0c 100644 --- a/app/javascript/packs/track-errors.ts +++ b/app/javascript/packs/track-errors.ts @@ -9,6 +9,6 @@ declare let window: WindowWithInitialErrors; const { _e: initialErrors } = window; const handleErrorEvent = (event: ErrorEvent) => - isTrackableErrorEvent(event) && trackError(event.error, event); + isTrackableErrorEvent(event) && trackError(event.error, { filename: event.filename }); initialErrors.forEach(handleErrorEvent); window.addEventListener('error', handleErrorEvent); diff --git a/app/javascript/packs/webauthn-setup.ts b/app/javascript/packs/webauthn-setup.ts index b0c2d0c9eff..2e28f36d8b6 100644 --- a/app/javascript/packs/webauthn-setup.ts +++ b/app/javascript/packs/webauthn-setup.ts @@ -75,7 +75,7 @@ function webauthn() { }) .catch((error: Error) => { if (!isExpectedWebauthnError(error)) { - trackError(error); + trackError(error, { errorId: 'webauthnSetup' }); } reloadWithError(error.name, { force: true }); diff --git a/app/services/frontend_error_logger.rb b/app/services/frontend_error_logger.rb index 3037954aa51..226dd49aff5 100644 --- a/app/services/frontend_error_logger.rb +++ b/app/services/frontend_error_logger.rb @@ -3,13 +3,13 @@ class FrontendErrorLogger class FrontendError < StandardError; end - def self.track_error(name:, message:, stack:, filename:) - return unless FrontendErrorForm.new.submit(filename:).success? + def self.track_error(name:, message:, stack:, filename: nil, error_id: nil) + return unless FrontendErrorForm.new.submit(filename:, error_id:).success? NewRelic::Agent.notice_error( FrontendError.new, expected: true, - custom_params: { frontend_error: { name:, message:, stack:, filename: } }, + custom_params: { frontend_error: { name:, message:, stack:, filename:, error_id: } }, ) end end diff --git a/docs/frontend.md b/docs/frontend.md index 61486c65a5e..50ea1687641 100644 --- a/docs/frontend.md +++ b/docs/frontend.md @@ -373,10 +373,14 @@ Each error includes a few details to help you debug: - `message`: Corresponds to [`Error#message`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/message), and is usually a good summary to group by - `name`: The subclass of the error (e.g. `TypeError`) - `stack`: A stacktrace of the individual error instance +- `filename`: The URL of the script where the error was raised, if it's an uncaught error +- `error_id`: A unique identifier for tracing caught errors explicitly tracked Note that NewRelic creates links in stack traces which are invalid, since they include the line and column number. If you encounter an "AccessDenied" error when clicking a stacktrace link, make sure to remove those details after the `.js` in your browser URL. -Debugging these stack traces can be difficult, since files in production are minified, and the stack traces include line numbers and columns for minified files. With the following steps, you can find a reference to the original code: +If an error includes `error_id`, you can use this to search in code for the corresponding call to `trackError` including that value as its `errorId` to trace where the error occurred. + +Otherwise, debugging these stack traces can be difficult, since files in production are minified, and the stack traces include line numbers and columns for minified files. With the following steps, you can find a reference to the original code: 1. Download the minified JavaScript file referenced in the stack trace - Example: https://secure.login.gov/packs/document-capture-e41c853e.digested.js diff --git a/spec/controllers/frontend_log_controller_spec.rb b/spec/controllers/frontend_log_controller_spec.rb index a717ac1ecd5..a891ce398b2 100644 --- a/spec/controllers/frontend_log_controller_spec.rb +++ b/spec/controllers/frontend_log_controller_spec.rb @@ -60,9 +60,11 @@ let(:flow_path) { 'standard' } let(:event) { 'IdV: location submitted' } let(:payload) do - { 'selected_location' => selected_location, + { + 'selected_location' => selected_location, 'flow_path' => flow_path, - 'opted_in_to_in_person_proofing' => nil } + 'opted_in_to_in_person_proofing' => nil, + } end it 'succeeds' do @@ -94,9 +96,11 @@ { opt_in_analytics_properties: true } end let(:payload) do - { 'selected_location' => selected_location, + { + 'selected_location' => selected_location, 'flow_path' => flow_path, - 'opted_in_to_in_person_proofing' => true } + 'opted_in_to_in_person_proofing' => true, + } end before do @@ -207,6 +211,7 @@ message: 'message', stack: 'stack', filename: 'filename', + error_id: nil, }, }, expected: true, diff --git a/spec/forms/frontend_error_form_spec.rb b/spec/forms/frontend_error_form_spec.rb index d800d1fe0df..189bcb1ef5b 100644 --- a/spec/forms/frontend_error_form_spec.rb +++ b/spec/forms/frontend_error_form_spec.rb @@ -1,8 +1,6 @@ require 'rails_helper' RSpec.describe FrontendErrorForm do - let(:filename) { 'https://example.com/foo.js' } - subject(:form) { described_class.new } before do @@ -10,7 +8,9 @@ end describe '#submit' do - subject(:result) { form.submit(filename:) } + subject(:result) { form.submit(filename:, error_id:) } + let(:error_id) { nil } + let(:filename) { 'https://example.com/foo.js' } context 'with valid filename' do let(:filename) { 'https://example.com/foo.js' } @@ -24,9 +24,20 @@ 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')]) + context 'without error id' do + 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 error id' do + let(:error_id) { 'exampleId' } + + it 'is successful' do + expect(result.success?).to eq(true) + expect(result.errors).to eq({}) + end end end diff --git a/spec/services/frontend_error_logger_spec.rb b/spec/services/frontend_error_logger_spec.rb index 7f1922e35eb..2e23ba28f1d 100644 --- a/spec/services/frontend_error_logger_spec.rb +++ b/spec/services/frontend_error_logger_spec.rb @@ -9,26 +9,51 @@ end describe '.track_event' do - it 'notices an expected error to NewRelic with custom parameters' do - expect(NewRelic::Agent).to receive(:notice_error).with( - kind_of(FrontendErrorLogger::FrontendError), - expected: true, - custom_params: { - frontend_error: { - name: 'name', - message: 'message', - stack: 'stack', - filename: 'filename.js', + let(:payload) { { name: 'name', message: 'message', stack: 'stack' } } + subject(:result) { FrontendErrorLogger.track_error(**payload) } + + context 'with filename payload' do + let(:payload) { super().merge(filename: 'filename.js') } + + it 'notices an expected error to NewRelic with custom parameters' do + expect(NewRelic::Agent).to receive(:notice_error).with( + kind_of(FrontendErrorLogger::FrontendError), + expected: true, + custom_params: { + frontend_error: { + name: 'name', + message: 'message', + stack: 'stack', + filename: 'filename.js', + error_id: nil, + }, + }, + ) + + result + end + end + + context 'with error id payload' do + let(:payload) { super().merge(error_id: 'exampleId') } + + it 'notices an expected error to NewRelic with custom parameters' do + expect(NewRelic::Agent).to receive(:notice_error).with( + kind_of(FrontendErrorLogger::FrontendError), + expected: true, + custom_params: { + frontend_error: { + name: 'name', + message: 'message', + stack: 'stack', + filename: nil, + error_id: 'exampleId', + }, }, - }, - ) - - FrontendErrorLogger.track_error( - name: 'name', - message: 'message', - stack: 'stack', - filename: 'filename.js', - ) + ) + + result + end end context 'with unsuccessful validation of request parameters' do