From d52066754f1541a49a0cc380351f1f15f564f3eb Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Fri, 21 Nov 2025 13:04:34 +0000 Subject: [PATCH 1/5] Completed error logging to Audit Also add logging upload to logging JS --- lib/GADS.pm | 3 + lib/GADS/API.pm | 24 ++++++- lib/GADS/Audit.pm | 21 ++++++- lib/GADS/Config.pm | 10 +++ src/frontend/js/lib/logging.js | 62 ++++++++++++++----- .../js/lib/util/scriptErrorHandler/index.ts | 35 +++++++++++ .../lib/MessageUploader.test.ts | 24 +++++++ .../scriptErrorHandler/lib/MessageUploader.ts | 33 ++++++++++ src/frontend/js/site.js | 3 +- views/layouts/main.tt | 2 +- 10 files changed, 196 insertions(+), 21 deletions(-) create mode 100644 src/frontend/js/lib/util/scriptErrorHandler/index.ts create mode 100644 src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.test.ts create mode 100644 src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.ts diff --git a/lib/GADS.pm b/lib/GADS.pm index 9592bb03b..0a692a062 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -346,6 +346,9 @@ hook before_template => sub { $tokens->{actions} = session 'actions'; session->delete('actions'); + my $config = GADS::Config->instance; + $tokens->{script_debug} = $config->has_log_level('SCRIPT'); + # This line used to be pre-request. However, occasionally errors have been # experienced with pages not submitting CSRF tokens. I think these may have # been race conditions where the session had been destroyed between the diff --git a/lib/GADS/API.pm b/lib/GADS/API.pm index fc1be173f..c4ff06094 100644 --- a/lib/GADS/API.pm +++ b/lib/GADS/API.pm @@ -1464,6 +1464,28 @@ get '/api/get_key' => require_login sub { } }; +post '/api/script_error' => require_login sub { + my $config = GADS::Config->instance; + if($config->has_log_level('SCRIPT')) { + my $user = logged_in_user; + my $body = _decode_json_body(); + + my $logger = GADS::Audit->new( + user => $user, + schema => schema, + ); + + $logger->script_error(%$body); + + content_type 'application/json; charset=UTF-8'; + return success "Script error logged successfully"; + } + else { + status 403; + error __"Logging of script errors is not enabled"; + } +}; + sub _success { my $msg = shift; send_as JSON => { @@ -1479,7 +1501,7 @@ sub _decode_json_body if request->content_type ne 'application/json'; my $body = try { decode_json request->body } - or error __"Failed to decode JSON"; + or error __"Failed to decode JSON" . " - " . $@; $body; } diff --git a/lib/GADS/Audit.pm b/lib/GADS/Audit.pm index f2da65450..946913503 100644 --- a/lib/GADS/Audit.pm +++ b/lib/GADS/Audit.pm @@ -70,7 +70,26 @@ has filtering => ( builder => sub { +{} }, ); -sub audit_types{ [qw/user_action login_change login_success logout login_failure/] }; +sub audit_types{ [qw/user_action login_change login_success logout login_failure script_error/] }; + +sub script_error +{ + my ($self, %options) = @_; + + my $description = $options{description} || 'Script error'; + my $method = $options{method} || 'unknown'; + my $url = $options{url} || 'unknown'; + + $self->schema->resultset('Audit')->create({ + user_id => $self->user_id, + description => $description, + type => 'script_error', + method => $method, + url => $url, + datetime => DateTime->now, + instance_id => $options{instance_id}, + }); +} sub user_action { my ($self, %options) = @_; diff --git a/lib/GADS/Config.pm b/lib/GADS/Config.pm index ca666df09..85e8e4a29 100644 --- a/lib/GADS/Config.pm +++ b/lib/GADS/Config.pm @@ -24,6 +24,8 @@ use Moo; with 'MooX::Singleton'; +sub TO_JSON { shift->gads } + has config => ( is => 'rw', required => 1, @@ -128,4 +130,12 @@ sub _build_uploads { $upload_path; }; +sub has_log_level { + my ($self, $level) = @_; + + my $levels = $self->config->{plugins}->{LogReport}->{session_messages} || []; + + return grep { $_ eq $level } @$levels; +} + 1; diff --git a/src/frontend/js/lib/logging.js b/src/frontend/js/lib/logging.js index bea4144c4..4799b610a 100644 --- a/src/frontend/js/lib/logging.js +++ b/src/frontend/js/lib/logging.js @@ -1,34 +1,62 @@ +import { uploadMessage } from "util/scriptErrorHandler"; + class Logging { - constructor() { - this.allowLogging = - window.test || - location.hostname === 'localhost' || - location.hostname === '127.0.0.1' || - location.hostname.endsWith('.peek.digitpaint.nl') + constructor() { + this.allowLogging = + window.test || + location.hostname === 'localhost' || + location.hostname === '127.0.0.1' || + location.hostname.endsWith('.peek.digitpaint.nl') } log(...message) { - if(this.allowLogging) { - console.log(message) - } + if (this.allowLogging) { + console.log(message) + } else { + const message = this.formatMessage('log', ...message) + uploadMessage(message) + } } info(...message) { - if(this.allowLogging) { - console.info(message) - } + if (this.allowLogging) { + console.info(message) + } else { + const message = this.formatMessage('info', ...message) + uploadMessage(message) + } } warn(...message) { - if(this.allowLogging) { - console.warn(message) - } + if (this.allowLogging) { + console.warn(message) + } else { + const message = this.formatMessage('warn', ...message) + uploadMessage(message) + } } error(...message) { - if(this.allowLogging) { - console.error(message) + if (this.allowLogging) { + console.error(message) + } else { + const message = this.formatMessage('error', ...message) + uploadMessage(message) + } + } + + formatMessage(type, ...message) { + let output = type + ': '; + for (let i = 0; i < message.length; i++) { + if (typeof message[i] === 'object') { + output += JSON.stringify(message[i]); + } else { + // This is wrapped so that anything that's not an object is converted to a string + output += `${message[i]}`; } + if (i < message.length - 1) output += ' '; + } + return output; } } diff --git a/src/frontend/js/lib/util/scriptErrorHandler/index.ts b/src/frontend/js/lib/util/scriptErrorHandler/index.ts new file mode 100644 index 000000000..85f088775 --- /dev/null +++ b/src/frontend/js/lib/util/scriptErrorHandler/index.ts @@ -0,0 +1,35 @@ +import { uploadMessage } from "./lib/MessageUploader"; + +if ($('body').data('script-debug')) { + console.warn("Script debug mode is enabled. Script errors will be uploaded."); + const createErrorString = (message: string, source: any, lineno: number, colno: number, error: Error | string | null) => { + let errorString = `Error: ${message}\nSource: ${source}\nLine: ${lineno}, Column: ${colno}`; + if (error && (error as Error)?.stack) { + errorString += `\nStack: ${(error as Error)?.stack}`; + } + return errorString; + } + + window.onerror = function (message: string, source: any, lineno: number, colno: number, error: Error | string | null) { + if (location.host === 'localhost') { + // If we're on localhost, we log the error to the console. This is useful for development. + console.error("Script error occurred:", message, source, lineno, colno, error); + } + if (location.pathname === '/api/script_error' || location.pathname === '/login') { + // If we're on the script error page, we don't want to log it again. + console.error("Script error occurred but not logged to avoid recursion."); + console.error(createErrorString(message, source, lineno, colno, error)); + return; + } + const description = createErrorString(message, source, lineno, colno, error) + console.error("Script error occurred:", description); + const method = 'N/A'; + + uploadMessage(description, method) + .catch(err => { + console.error("Failed to upload script error:", err); + }); + } +} + +export { uploadMessage }; diff --git a/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.test.ts b/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.test.ts new file mode 100644 index 000000000..ad82189bd --- /dev/null +++ b/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.test.ts @@ -0,0 +1,24 @@ +import { describe, it, expect, jest } from '@jest/globals'; +import { MessageUploader } from './MessageUploader'; + +describe('MessageUploader', () => { + class MockUploader { + upload = jest.fn(); + } + + it('should upload messages correctly', async () => { + const uploader = new MockUploader(); + const messageUploader = new MessageUploader(uploader); + + const messages = { id: 1, content: 'Test message 1' }; + + await messageUploader.uploadMessage(JSON.stringify(messages)); + + expect(uploader.upload).toHaveBeenCalledTimes(1); + expect(uploader.upload).toHaveBeenCalledWith({ + description: JSON.stringify(messages), + method: 'N/A', + url: 'http://localhost/' + }); + }); +}); \ No newline at end of file diff --git a/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.ts b/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.ts new file mode 100644 index 000000000..bc3a88952 --- /dev/null +++ b/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.ts @@ -0,0 +1,33 @@ +import { Uploader } from "util/upload/UploadControl"; + +export const uploadMessage = async (message: string, method?: string) => { + const body = { + description: message, + method: method || 'N/A', + url: window.location.href + }; + const uploader = new Uploader('/api/script_error', 'POST'); + const messageUploader = new MessageUploader(uploader); + return await messageUploader.uploadMessage(body.description, body.method); +}; + +export class MessageUploader { + constructor(private uploader: Uploader) { + } + + async uploadMessage(description: string, method?: string): Promise { + method ||= 'N/A'; + const csrf_token = document.body.dataset.csrf; + const body = { + description, + method, + url: window.location.href, + csrf_token + }; + try { + return await this.uploader.upload(body); + } catch (err) { + console.error("Failed to upload message:", err); + } + } +} \ No newline at end of file diff --git a/src/frontend/js/site.js b/src/frontend/js/site.js index 1517accdf..e93f46bbc 100644 --- a/src/frontend/js/site.js +++ b/src/frontend/js/site.js @@ -4,6 +4,7 @@ import 'bootstrap'; import 'components/graph/lib/chart'; import 'util/filedrag'; import 'util/actionsHandler'; +import 'util/scriptErrorHandler'; // Components import AddTableModalComponent from 'components/modal/modals/new-table'; @@ -87,4 +88,4 @@ registerComponent(AutosaveComponent); // Initialize all components at some point initializeRegisteredComponents(document.body); -handleActions(); \ No newline at end of file +handleActions(); diff --git a/views/layouts/main.tt b/views/layouts/main.tt index dd4bf1c73..0b112bd52 100755 --- a/views/layouts/main.tt +++ b/views/layouts/main.tt @@ -9,7 +9,7 @@ - +
[% IF page == "login" OR page == "register" OR page == "reset" OR page == "invalidsite"; From 612a083379f03748fe94e74bc6a0688ae3b2ea21 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Thu, 5 Mar 2026 13:26:31 +0000 Subject: [PATCH 2/5] Update to use `Log::Report` --- lib/GADS.pm | 3 -- lib/GADS/API.pm | 23 ++------ lib/GADS/Audit.pm | 21 +------- lib/GADS/Config.pm | 10 ---- .../js/lib/util/scriptErrorHandler/index.ts | 52 +++++++++---------- views/layouts/main.tt | 2 +- 6 files changed, 32 insertions(+), 79 deletions(-) diff --git a/lib/GADS.pm b/lib/GADS.pm index 0a692a062..9592bb03b 100644 --- a/lib/GADS.pm +++ b/lib/GADS.pm @@ -346,9 +346,6 @@ hook before_template => sub { $tokens->{actions} = session 'actions'; session->delete('actions'); - my $config = GADS::Config->instance; - $tokens->{script_debug} = $config->has_log_level('SCRIPT'); - # This line used to be pre-request. However, occasionally errors have been # experienced with pages not submitting CSRF tokens. I think these may have # been race conditions where the session had been destroyed between the diff --git a/lib/GADS/API.pm b/lib/GADS/API.pm index c4ff06094..d333e8ab5 100644 --- a/lib/GADS/API.pm +++ b/lib/GADS/API.pm @@ -1465,25 +1465,12 @@ get '/api/get_key' => require_login sub { }; post '/api/script_error' => require_login sub { - my $config = GADS::Config->instance; - if($config->has_log_level('SCRIPT')) { - my $user = logged_in_user; - my $body = _decode_json_body(); - - my $logger = GADS::Audit->new( - user => $user, - schema => schema, - ); + my $body = _decode_json_body(); - $logger->script_error(%$body); + info "SCRIPT ERROR: " . $body->{url} . " - " . $body->{description}; - content_type 'application/json; charset=UTF-8'; - return success "Script error logged successfully"; - } - else { - status 403; - error __"Logging of script errors is not enabled"; - } + content_type 'application/json; charset=UTF-8'; + return success "Script error logged successfully"; }; sub _success @@ -1501,7 +1488,7 @@ sub _decode_json_body if request->content_type ne 'application/json'; my $body = try { decode_json request->body } - or error __"Failed to decode JSON" . " - " . $@; + or error __"Failed to decode JSON"; $body; } diff --git a/lib/GADS/Audit.pm b/lib/GADS/Audit.pm index 946913503..f2da65450 100644 --- a/lib/GADS/Audit.pm +++ b/lib/GADS/Audit.pm @@ -70,26 +70,7 @@ has filtering => ( builder => sub { +{} }, ); -sub audit_types{ [qw/user_action login_change login_success logout login_failure script_error/] }; - -sub script_error -{ - my ($self, %options) = @_; - - my $description = $options{description} || 'Script error'; - my $method = $options{method} || 'unknown'; - my $url = $options{url} || 'unknown'; - - $self->schema->resultset('Audit')->create({ - user_id => $self->user_id, - description => $description, - type => 'script_error', - method => $method, - url => $url, - datetime => DateTime->now, - instance_id => $options{instance_id}, - }); -} +sub audit_types{ [qw/user_action login_change login_success logout login_failure/] }; sub user_action { my ($self, %options) = @_; diff --git a/lib/GADS/Config.pm b/lib/GADS/Config.pm index 85e8e4a29..ca666df09 100644 --- a/lib/GADS/Config.pm +++ b/lib/GADS/Config.pm @@ -24,8 +24,6 @@ use Moo; with 'MooX::Singleton'; -sub TO_JSON { shift->gads } - has config => ( is => 'rw', required => 1, @@ -130,12 +128,4 @@ sub _build_uploads { $upload_path; }; -sub has_log_level { - my ($self, $level) = @_; - - my $levels = $self->config->{plugins}->{LogReport}->{session_messages} || []; - - return grep { $_ eq $level } @$levels; -} - 1; diff --git a/src/frontend/js/lib/util/scriptErrorHandler/index.ts b/src/frontend/js/lib/util/scriptErrorHandler/index.ts index 85f088775..32b6535d1 100644 --- a/src/frontend/js/lib/util/scriptErrorHandler/index.ts +++ b/src/frontend/js/lib/util/scriptErrorHandler/index.ts @@ -1,35 +1,33 @@ import { uploadMessage } from "./lib/MessageUploader"; -if ($('body').data('script-debug')) { - console.warn("Script debug mode is enabled. Script errors will be uploaded."); - const createErrorString = (message: string, source: any, lineno: number, colno: number, error: Error | string | null) => { - let errorString = `Error: ${message}\nSource: ${source}\nLine: ${lineno}, Column: ${colno}`; - if (error && (error as Error)?.stack) { - errorString += `\nStack: ${(error as Error)?.stack}`; - } - return errorString; +const createErrorString = (message: string, source: any, lineno: number, colno: number, error: Error | string | null) => { + let errorString = `Error: ${message}\nSource: ${source}\nLine: ${lineno}, Column: ${colno}`; + if (error && (error as Error)?.stack) { + errorString += `\nStack: ${(error as Error)?.stack}`; } + return errorString; +} - window.onerror = function (message: string, source: any, lineno: number, colno: number, error: Error | string | null) { - if (location.host === 'localhost') { - // If we're on localhost, we log the error to the console. This is useful for development. - console.error("Script error occurred:", message, source, lineno, colno, error); - } - if (location.pathname === '/api/script_error' || location.pathname === '/login') { - // If we're on the script error page, we don't want to log it again. - console.error("Script error occurred but not logged to avoid recursion."); - console.error(createErrorString(message, source, lineno, colno, error)); - return; - } - const description = createErrorString(message, source, lineno, colno, error) - console.error("Script error occurred:", description); - const method = 'N/A'; - - uploadMessage(description, method) - .catch(err => { - console.error("Failed to upload script error:", err); - }); +window.onerror = function (message: string, source: any, lineno: number, colno: number, error: Error | string | null) { + if (location.host === 'localhost') { + // If we're on localhost, we log the error to the console. This is useful for development. + console.error("Script error occurred:", message, source, lineno, colno, error); + } + if (location.pathname === '/api/script_error' || location.pathname === '/login') { + // If we're on the script error page, we don't want to log it again. + console.error("Script error occurred but not logged to avoid recursion."); + console.error(createErrorString(message, source, lineno, colno, error)); + return; } + + const description = createErrorString(message, source, lineno, colno, error) + console.error("Script error occurred:", description); + const method = 'N/A'; + + uploadMessage(description, method) + .catch(err => { + console.error("Failed to upload script error:", err); + }); } export { uploadMessage }; diff --git a/views/layouts/main.tt b/views/layouts/main.tt index 0b112bd52..dd4bf1c73 100755 --- a/views/layouts/main.tt +++ b/views/layouts/main.tt @@ -9,7 +9,7 @@ - +
[% IF page == "login" OR page == "register" OR page == "reset" OR page == "invalidsite"; From 28c2e7f823943c4263c252700d68fa9b2c6a28b4 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Thu, 5 Mar 2026 13:47:05 +0000 Subject: [PATCH 3/5] Change code as per review Also remove extrenous logging and debugging code that was added during development of this feature. --- lib/GADS/API.pm | 5 +++-- src/frontend/js/lib/util/scriptErrorHandler/index.ts | 4 +--- .../util/scriptErrorHandler/lib/MessageUploader.ts | 11 ++++------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/lib/GADS/API.pm b/lib/GADS/API.pm index d333e8ab5..dc165f2d4 100644 --- a/lib/GADS/API.pm +++ b/lib/GADS/API.pm @@ -1467,10 +1467,11 @@ get '/api/get_key' => require_login sub { post '/api/script_error' => require_login sub { my $body = _decode_json_body(); - info "SCRIPT ERROR: " . $body->{url} . " - " . $body->{description}; + info __x "SCRIPT ERROR: {url} - {description}", + url => $body->{url}, description => $body->{description}; content_type 'application/json; charset=UTF-8'; - return success "Script error logged successfully"; + return "Script error logged successfully"; }; sub _success diff --git a/src/frontend/js/lib/util/scriptErrorHandler/index.ts b/src/frontend/js/lib/util/scriptErrorHandler/index.ts index 32b6535d1..47cd1b857 100644 --- a/src/frontend/js/lib/util/scriptErrorHandler/index.ts +++ b/src/frontend/js/lib/util/scriptErrorHandler/index.ts @@ -21,10 +21,8 @@ window.onerror = function (message: string, source: any, lineno: number, colno: } const description = createErrorString(message, source, lineno, colno, error) - console.error("Script error occurred:", description); - const method = 'N/A'; - uploadMessage(description, method) + uploadMessage(description) .catch(err => { console.error("Failed to upload script error:", err); }); diff --git a/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.ts b/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.ts index bc3a88952..b362cfeb7 100644 --- a/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.ts +++ b/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.ts @@ -1,26 +1,23 @@ import { Uploader } from "util/upload/UploadControl"; -export const uploadMessage = async (message: string, method?: string) => { +export const uploadMessage = async (message: string) => { const body = { description: message, - method: method || 'N/A', url: window.location.href }; const uploader = new Uploader('/api/script_error', 'POST'); const messageUploader = new MessageUploader(uploader); - return await messageUploader.uploadMessage(body.description, body.method); + return await messageUploader.uploadMessage(body.description); }; export class MessageUploader { constructor(private uploader: Uploader) { } - async uploadMessage(description: string, method?: string): Promise { - method ||= 'N/A'; + async uploadMessage(description: string): Promise { const csrf_token = document.body.dataset.csrf; const body = { description, - method, url: window.location.href, csrf_token }; @@ -30,4 +27,4 @@ export class MessageUploader { console.error("Failed to upload message:", err); } } -} \ No newline at end of file +} From 6bcca4c420db448f87787fed0fb8a3ec17b8f841 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Thu, 5 Mar 2026 13:50:36 +0000 Subject: [PATCH 4/5] Fix erroring test --- .../js/lib/util/scriptErrorHandler/lib/MessageUploader.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.test.ts b/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.test.ts index ad82189bd..b7725c100 100644 --- a/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.test.ts +++ b/src/frontend/js/lib/util/scriptErrorHandler/lib/MessageUploader.test.ts @@ -17,8 +17,7 @@ describe('MessageUploader', () => { expect(uploader.upload).toHaveBeenCalledTimes(1); expect(uploader.upload).toHaveBeenCalledWith({ description: JSON.stringify(messages), - method: 'N/A', url: 'http://localhost/' }); }); -}); \ No newline at end of file +}); From 5c932755b1353ce2c8a1a1c233745169ce0178b5 Mon Sep 17 00:00:00 2001 From: Dave Roberts Date: Thu, 5 Mar 2026 15:30:45 +0000 Subject: [PATCH 5/5] Return JSON as required --- lib/GADS/API.pm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/GADS/API.pm b/lib/GADS/API.pm index dc165f2d4..43a484215 100644 --- a/lib/GADS/API.pm +++ b/lib/GADS/API.pm @@ -1470,8 +1470,7 @@ post '/api/script_error' => require_login sub { info __x "SCRIPT ERROR: {url} - {description}", url => $body->{url}, description => $body->{description}; - content_type 'application/json; charset=UTF-8'; - return "Script error logged successfully"; + _success("Script error logged successfully"); }; sub _success