Enhance error handling and logging in QRS utility functions#1645
Merged
Conversation
mountaindude
commented
Jun 20, 2026
Collaborator
- Introduced detailed error formatting in task metadata, task tags, and user sync task execution results functions.
- Added checks for expected QRS response statuses and improved logging for unexpected responses.
- Created a new module for QRS error handling with functions to format errors and results with context.
- Added unit tests for the new error handling functions to ensure proper functionality and coverage.
- Documented the new error handling capabilities in the relevant skill markdown files.
…tions - Introduced detailed error formatting in task metadata, task tags, and user sync task execution results functions. - Added checks for expected QRS response statuses and improved logging for unexpected responses. - Created a new module for QRS error handling with functions to format errors and results with context. - Added unit tests for the new error handling functions to ensure proper functionality and coverage. - Documented the new error handling capabilities in the relevant skill markdown files.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes QRS/HTTP error handling across Butler by introducing a shared formatting/validation helper (src/lib/qrs_error.js) and wiring it into multiple QRS utility functions and QSEoW modules, with expanded unit test coverage and updated documentation/skills.
Changes:
- Added
src/lib/qrs_error.jsto format QRS/HTTP errors/results with request context and to validate expected status codes. - Updated multiple QRS utility/QSEoW modules to validate QRS responses before accessing payloads and to log richer context on failures.
- Added/updated unit tests for the new helper and for the updated QRS utility functions; refreshed related documentation/skill markdown files.
Reviewed changes
Copilot reviewed 42 out of 59 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/qrs_util/usersync_task_execution_results.js | Use shared QRS response validation + contextual error formatting. |
| src/qrs_util/task_tag_util.js | Validate task/full responses and format QRS errors/results consistently. |
| src/qrs_util/task_metadata.js | Validate task/full responses and format QRS errors/results consistently. |
| src/qrs_util/task_cp_util.js | Validate QRS responses for CP queries; format errors/results with context. |
| src/qrs_util/sense_start_task.js | Improve non-204 logging with structured QRS response context. |
| src/qrs_util/reload_task_execution_results.js | Validate reload task lookup response shape/status before use. |
| src/qrs_util/preload_task_execution_results.js | Validate preload task lookup response shape/status before use. |
| src/qrs_util/get_tasks.js | Validate QRS responses for tag/CP filters; structured error formatting. |
| src/qrs_util/get_app_owner.js | Validate step 1/2 QRS responses before extracting owner/attributes. |
| src/qrs_util/externalprogram_task_execution_results.js | Validate response shape/status; structured error formatting. |
| src/qrs_util/does_task_exist.js | Validate response shape/status; structured error formatting. |
| src/qrs_util/distribute_task_execution_results.js | Validate response shape/status; structured error formatting. |
| src/qrs_util/app_tag_util.js | Validate response shape/status and tags payload before mapping. |
| src/qrs_util/app_metadata.js | Validate response shape/status before returning metadata. |
| src/qrs_util/tests/usersync_task_execution_results.test.js | Add coverage for unexpected HTTP status handling; adjust mocks. |
| src/qrs_util/tests/task_tag_util.test.js | Add coverage for unexpected non-200 QRS response handling. |
| src/qrs_util/tests/task_metadata.test.js | Add coverage for unexpected non-200 QRS response handling. |
| src/qrs_util/tests/task_cp_util.test.js | Add coverage for unexpected non-200 responses across helpers. |
| src/qrs_util/tests/sense_start_task.test.js | Assert structured logging contains expected/actual status metadata. |
| src/qrs_util/tests/reload_task_execution_results.test.js | New test suite for reload task execution results + error cases. |
| src/qrs_util/tests/preload_task_execution_results.test.js | Add coverage for unexpected HTTP status handling; adjust mocks. |
| src/qrs_util/tests/get_tasks.test.js | Add coverage for unexpected non-200 QRS response handling. |
| src/qrs_util/tests/get_app_owner.test.js | Add coverage for non-200 response handling in both steps. |
| src/qrs_util/tests/externalprogram_task_execution_results.test.js | Add coverage for non-200 responses; verify contextual logging. |
| src/qrs_util/tests/does_task_exist.test.js | Add coverage for unexpected non-200 response handling. |
| src/qrs_util/tests/distribute_task_execution_results.test.js | Add coverage for unexpected HTTP status handling; adjust mocks. |
| src/qrs_util/tests/app_tag_util.test.js | Add coverage for unexpected non-200 QRS response handling. |
| src/qrs_util/tests/app_metadata.test.js | Add coverage for unexpected non-200 QRS response handling. |
| src/lib/qseow/scriptlog.js | Use shared helpers to validate and log unexpected QRS responses during script log retrieval. |
| src/lib/qseow/qliksense_version.js | Apply shared HTTP formatting/validation for version monitor’s /v1/systeminfo call. |
| src/lib/qseow/qliksense_license.js | Replace ad-hoc formatting with shared QRS helpers and response validation. |
| src/lib/qseow/tests/scriptlog.test.js | Add coverage for deprecated reference lookup non-200 fallback behavior; adjust mocks. |
| src/lib/qseow/tests/qliksense_version.test.js | Add coverage for structured logging on unexpected HTTP responses and thrown errors. |
| src/lib/qrs_error.js | New shared QRS/HTTP formatting + status validation utilities. |
| src/lib/incident_mgmt/new_relic.js | Extract CP-account lookup into helper with structured QRS validation/logging. |
| src/lib/incident_mgmt/tests/new_relic.test.js | Add coverage for non-200 CP lookup logging path. |
| src/lib/assert/assert_config_file.js | Add structured QRS validation/logging when asserting New Relic CP configuration. |
| src/lib/assert/tests/assert_config_file.test.js | Add test ensuring assert fails on non-200 QRS custom property lookup. |
| src/lib/tests/qrs_error.test.js | New unit tests for qrs_error helper behavior and redaction. |
| docs/to-doc-site/qrs-api-error-messages.md | Document the new structured error patterns and where they apply. |
| CLAUDE.md | Update GitNexus index metadata references. |
| AGENTS.md | Update GitNexus index metadata references. |
| .claude/skills/generated/udp/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/smtp/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/rest-server/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/qseow/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/qrs-util/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/influxdb/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/incident-mgmt/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/handlers/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/get/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/cluster-32/SKILL.md | Add new generated cluster skill metadata. |
| .claude/skills/generated/cluster-30/SKILL.md | Refresh generated cluster skill metadata/line mappings. |
| .claude/skills/generated/cluster-29/SKILL.md | Add new generated cluster skill metadata. |
| .claude/skills/generated/cluster-28/SKILL.md | Remove outdated generated cluster skill metadata. |
| .claude/skills/generated/cluster-27/SKILL.md | Refresh generated cluster skill metadata/line mappings. |
| .claude/skills/generated/cluster-25/SKILL.md | Remove outdated generated cluster skill metadata. |
| .claude/skills/generated/assert/SKILL.md | Refresh generated skill metadata/line mappings. |
| .claude/skills/generated/api/SKILL.md | Refresh generated skill metadata/line mappings. |
| })}`, | ||
| ); | ||
| return false; | ||
| } else |
Comment on lines
+72
to
+75
| if (typeof body === 'string' || typeof body === 'number' || typeof body === 'boolean') { | ||
| parts.push(`body: ${body}`); | ||
| return; | ||
| } |
Co-authored-by: mountaindude <1029262+mountaindude@users.noreply.github.com>
Comment on lines
42
to
+44
| if (filter.tag) { | ||
| globals.logger.debug(`GETTASKS 1: task/full?filter=tags.name eq '${filter.tag}'`); | ||
| endpoint = `task/full?filter=tags.name eq '${filter.tag}'`; | ||
| globals.logger.debug(`GETTASKS 1: ${endpoint}`); |
Comment on lines
71
to
+73
| if (filter.customProperty) { | ||
| globals.logger.debug( | ||
| `GETTASKS 2: task/full?filter=(customProperties.definition.name eq '${filter.customProperty.name}') and (customProperties.value eq '${filter.customProperty.value}')`, | ||
| ); | ||
| endpoint = `task/full?filter=(customProperties.definition.name eq '${filter.customProperty.name}') and (customProperties.value eq '${filter.customProperty.value}')`; | ||
| globals.logger.debug(`GETTASKS 2: ${endpoint}`); |
Comment on lines
+38
to
+42
| if (!hasExpectedQrsStatus(result) || !result.body?.owner) { | ||
| globals.logger.error( | ||
| `APPOWNER: Unexpected app owner response: ${formatQrsResultWithContext(result, appEndpoint, qrsConfig, { | ||
| method: 'GET', | ||
| expectedStatusCodes: [200], |
Comment on lines
+45
to
+49
| const originalMockResolvedValue = mockQrsClient.Get.mockResolvedValue.bind(mockQrsClient.Get); | ||
| const originalMockResolvedValueOnce = mockQrsClient.Get.mockResolvedValueOnce.bind(mockQrsClient.Get); | ||
|
|
||
| mockQrsClient.Get.mockResolvedValue = (value) => originalMockResolvedValue({ statusCode: 200, ...value }); | ||
| mockQrsClient.Get.mockResolvedValueOnce = (value) => originalMockResolvedValueOnce({ statusCode: 200, ...value }); |
Comment on lines
+88
to
+92
| const originalMockResolvedValue = mockQrsGet.mockResolvedValue.bind(mockQrsGet); | ||
| const originalMockResolvedValueOnce = mockQrsGet.mockResolvedValueOnce.bind(mockQrsGet); | ||
|
|
||
| mockQrsGet.mockResolvedValue = (value) => originalMockResolvedValue({ statusCode: 200, ...value }); | ||
| mockQrsGet.mockResolvedValueOnce = (value) => originalMockResolvedValueOnce({ statusCode: 200, ...value }); |
Comment on lines
+104
to
+108
| const originalMockResolvedValue = mockQrsGet.mockResolvedValue.bind(mockQrsGet); | ||
| const originalMockResolvedValueOnce = mockQrsGet.mockResolvedValueOnce.bind(mockQrsGet); | ||
|
|
||
| mockQrsGet.mockResolvedValue = (value) => originalMockResolvedValue({ statusCode: 200, ...value }); | ||
| mockQrsGet.mockResolvedValueOnce = (value) => originalMockResolvedValueOnce({ statusCode: 200, ...value }); |
Comment on lines
+67
to
+71
| const originalMockResolvedValue = qrsGetMock.mockResolvedValue.bind(qrsGetMock); | ||
| const originalMockResolvedValueOnce = qrsGetMock.mockResolvedValueOnce.bind(qrsGetMock); | ||
|
|
||
| qrsGetMock.mockResolvedValue = (value) => originalMockResolvedValue({ statusCode: 200, ...value }); | ||
| qrsGetMock.mockResolvedValueOnce = (value) => originalMockResolvedValueOnce({ statusCode: 200, ...value }); |
Co-authored-by: mountaindude <1029262+mountaindude@users.noreply.github.com>
Comment on lines
42
to
+44
| if (filter.tag) { | ||
| globals.logger.debug(`GETTASKS 1: task/full?filter=tags.name eq '${filter.tag}'`); | ||
| endpoint = `task/full?filter=tags.name eq '${filter.tag}'`; | ||
| globals.logger.debug(`GETTASKS 1: ${endpoint}`); |
Comment on lines
+17
to
+18
| let qrsConfig; | ||
| const endpoint = `task/full?filter=id eq ${taskId} and customProperties.definition.name eq '${cpName}' and customProperties.value eq '${cpValue}'`; |
Comment on lines
+82
to
+84
| let qrsConfig; | ||
| const endpoint = `task/full?filter=id eq ${taskId} and customProperties.definition.name eq '${cpName}'`; | ||
|
|
Comment on lines
+45
to
+49
| const originalMockResolvedValue = mockQrsClient.Get.mockResolvedValue.bind(mockQrsClient.Get); | ||
| const originalMockResolvedValueOnce = mockQrsClient.Get.mockResolvedValueOnce.bind(mockQrsClient.Get); | ||
|
|
||
| mockQrsClient.Get.mockResolvedValue = (value) => originalMockResolvedValue({ statusCode: 200, ...value }); | ||
| mockQrsClient.Get.mockResolvedValueOnce = (value) => originalMockResolvedValueOnce({ statusCode: 200, ...value }); |
Comment on lines
+88
to
+92
| const originalMockResolvedValue = mockQrsGet.mockResolvedValue.bind(mockQrsGet); | ||
| const originalMockResolvedValueOnce = mockQrsGet.mockResolvedValueOnce.bind(mockQrsGet); | ||
|
|
||
| mockQrsGet.mockResolvedValue = (value) => originalMockResolvedValue({ statusCode: 200, ...value }); | ||
| mockQrsGet.mockResolvedValueOnce = (value) => originalMockResolvedValueOnce({ statusCode: 200, ...value }); |
Comment on lines
+104
to
+108
| const originalMockResolvedValue = mockQrsGet.mockResolvedValue.bind(mockQrsGet); | ||
| const originalMockResolvedValueOnce = mockQrsGet.mockResolvedValueOnce.bind(mockQrsGet); | ||
|
|
||
| mockQrsGet.mockResolvedValue = (value) => originalMockResolvedValue({ statusCode: 200, ...value }); | ||
| mockQrsGet.mockResolvedValueOnce = (value) => originalMockResolvedValueOnce({ statusCode: 200, ...value }); |
Comment on lines
+67
to
+71
| const originalMockResolvedValue = qrsGetMock.mockResolvedValue.bind(qrsGetMock); | ||
| const originalMockResolvedValueOnce = qrsGetMock.mockResolvedValueOnce.bind(qrsGetMock); | ||
|
|
||
| qrsGetMock.mockResolvedValue = (value) => originalMockResolvedValue({ statusCode: 200, ...value }); | ||
| qrsGetMock.mockResolvedValueOnce = (value) => originalMockResolvedValueOnce({ statusCode: 200, ...value }); |
Co-authored-by: mountaindude <1029262+mountaindude@users.noreply.github.com>
Comment on lines
42
to
+45
| if (filter.tag) { | ||
| globals.logger.debug(`GETTASKS 1: task/full?filter=tags.name eq '${filter.tag}'`); | ||
| const tagName = filter.tag.replaceAll("'", "''"); | ||
| endpoint = `task/full?filter=tags.name eq '${tagName}'`; | ||
| globals.logger.debug(`GETTASKS 1: ${endpoint}`); |
Comment on lines
72
to
+76
| if (filter.customProperty) { | ||
| globals.logger.debug( | ||
| `GETTASKS 2: task/full?filter=(customProperties.definition.name eq '${filter.customProperty.name}') and (customProperties.value eq '${filter.customProperty.value}')`, | ||
| ); | ||
| const customPropertyName = filter.customProperty.name.replaceAll("'", "''"); | ||
| const customPropertyValue = filter.customProperty.value.replaceAll("'", "''"); | ||
| endpoint = `task/full?filter=(customProperties.definition.name eq '${customPropertyName}') and (customProperties.value eq '${customPropertyValue}')`; | ||
| globals.logger.debug(`GETTASKS 2: ${endpoint}`); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: mountaindude <1029262+mountaindude@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
74
to
76
| } catch (err) { | ||
| globals.logger.error(`GETAPPTAGS: Error while getting tags: ${globals.getErrorMessage(err)}`); | ||
| globals.logger.error(`GETAPPTAGS: Error while getting tags: ${formatQrsErrorWithContext(err, endpoint, qrsConfig)}`); | ||
| return false; |
Comment on lines
+14
to
+24
| function getVersionMonitorRequestContext() { | ||
| const hostname = globals.config.get('Butler.qlikSenseVersion.versionMonitor.host'); | ||
| const portNumber = 9032; | ||
|
|
||
| return { | ||
| hostname, | ||
| portNumber, | ||
| baseURL: `https://${hostname}:${portNumber}`, | ||
| timeout: HTTP_TIMEOUT_SHORT_MS, | ||
| }; | ||
| } |
Comment on lines
31
to
41
| @@ -21,9 +41,9 @@ async function checkQlikSenseVersion(config, logger) { | |||
| }); | |||
Comment on lines
109
to
+114
| } catch (err) { | ||
| if (globals.isSea) { | ||
| logger.error(`[QSEOW] QLIKSENSE VERSION MONITOR INIT: ${globals.getErrorMessage(err)}`); | ||
| } else { | ||
| logger.error(`[QSEOW] QLIKSENSE VERSION MONITOR INIT: ${globals.getErrorMessage(err)}`); | ||
| } | ||
| logger.error( | ||
| `[QSEOW] QLIKSENSE VERSION MONITOR INIT: ${formatHttpErrorWithContext(err, '/v1/systeminfo', getVersionMonitorRequestContext(), { | ||
| method: 'GET', | ||
| })}`, | ||
| ); |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


