From ead9863ea79a857cc4b4e2bf82f4f36ade3b0bf2 Mon Sep 17 00:00:00 2001 From: Jose Costa Teixeira <16153168+costateixeira@users.noreply.github.com> Date: Thu, 19 Feb 2026 18:34:00 +0100 Subject: [PATCH 1/2] Improve error handling for source loading failures Sources that fail to fetch or load are now logged and skipped instead of crashing the server. Errors are written to both console and log file so they are visible in Docker output (Winston suppresses errors from console by default). A summary of failed sources is logged after loading completes. Co-Authored-By: Claude --- library/package-manager.js | 69 +++++++------- tests/tx/library-error-handling.test.js | 121 ++++++++++++++++++++++++ tx/library.js | 35 ++++++- tx/library/codesystem.js | 7 +- 4 files changed, 196 insertions(+), 36 deletions(-) create mode 100644 tests/tx/library-error-handling.test.js diff --git a/library/package-manager.js b/library/package-manager.js index 00693d1..5ac4fc7 100644 --- a/library/package-manager.js +++ b/library/package-manager.js @@ -594,41 +594,46 @@ class PackageManager { */ async fetchUrl(url) { console.log("Fetch Package from URL: " + url); - const client = new CIBuildClient(); - const packageData = await client.fetchFromUrlSpecific(url); - - // Extract to a temp location to read package.json for name and version - const tempKey = `_url_temp_${Date.now()}`; - const tempPath = await this.extractToCache(tempKey, 'url', packageData); - const tempFullPath = path.join(this.cacheFolder, tempPath); - - // Read package name and version from the extracted package - const pkgJsonPath = path.join(tempFullPath, 'package', 'package.json'); - const pkgJson = JSON.parse(await fs.readFile(pkgJsonPath, 'utf8')); - const packageId = pkgJson.name; - const version = pkgJson.version; - - if (!packageId || !version) { - throw new Error(`Package at ${url} has no name or version in package.json`); - } + try { + const client = new CIBuildClient(); + const packageData = await client.fetchFromUrlSpecific(url); + + // Extract to a temp location to read package.json for name and version + const tempKey = `_url_temp_${Date.now()}`; + const tempPath = await this.extractToCache(tempKey, 'url', packageData); + const tempFullPath = path.join(this.cacheFolder, tempPath); + + // Read package name and version from the extracted package + const pkgJsonPath = path.join(tempFullPath, 'package', 'package.json'); + const pkgJson = JSON.parse(await fs.readFile(pkgJsonPath, 'utf8')); + const packageId = pkgJson.name; + const version = pkgJson.version; + + if (!packageId || !version) { + throw new Error(`Package at ${url} has no name or version in package.json`); + } - // Use the same cache key format as npm packages - const finalName = `${packageId}#${version}`; - const finalPath = path.join(this.cacheFolder, finalName); + // Use the same cache key format as npm packages + const finalName = `${packageId}#${version}`; + const finalPath = path.join(this.cacheFolder, finalName); - // If it already exists, the same package is already loaded - that's a config error - try { - await fs.access(finalPath); - await fs.rm(tempFullPath, { recursive: true, force: true }); - throw new Error(`Package ${finalName} already exists in cache. Check library config for duplicates (url: ${url})`); - } catch (e) { - if (e.message.includes('already exists')) throw e; - // Doesn't exist yet, rename temp to final - await fs.rename(tempFullPath, finalPath); - } + // If it already exists, the same package is already loaded - that's a config error + try { + await fs.access(finalPath); + await fs.rm(tempFullPath, { recursive: true, force: true }); + throw new Error(`Package ${finalName} already exists in cache. Check library config for duplicates (url: ${url})`); + } catch (e) { + if (e.message.includes('already exists')) throw e; + // Doesn't exist yet, rename temp to final + await fs.rename(tempFullPath, finalPath); + } - this.totalDownloaded = this.totalDownloaded + packageData.length; - return finalName; + this.totalDownloaded = this.totalDownloaded + packageData.length; + return finalName; + } catch (error) { + console.error(`Error fetching package from URL ${url}: ${error.message}`); + throw error; + } } /** diff --git a/tests/tx/library-error-handling.test.js b/tests/tx/library-error-handling.test.js new file mode 100644 index 0000000..856180d --- /dev/null +++ b/tests/tx/library-error-handling.test.js @@ -0,0 +1,121 @@ +const { Library } = require('../../tx/library'); +const path = require('path'); +const fs = require('fs').promises; +const os = require('os'); + +describe('Library error handling', () => { + let tmpDir; + let yamlPath; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'lib-test-')); + yamlPath = path.join(tmpDir, 'library.yml'); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + function createLibrary(configFile) { + const log = { + info: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + debug: jest.fn(), + }; + const stats = { addStat: jest.fn() }; + return { library: new Library(configFile, undefined, log, stats), log }; + } + + test('failed source does not prevent other sources from loading', async () => { + // Config with 3 internal sources (no network needed) + await fs.writeFile(yamlPath, [ + 'base:', + ' url: https://storage.googleapis.com/tx-fhir-org', + 'sources:', + ' - internal:lang', + ' - internal:INVALID_SOURCE', + ' - internal:country', + ].join('\n')); + + const { library, log } = createLibrary(yamlPath); + await library.load(); + + // The valid sources should have loaded; the invalid one should have been skipped + const errorCalls = log.error.mock.calls.map(c => c[0]); + const hasFailure = errorCalls.some(msg => msg.includes('INVALID_SOURCE')); + expect(hasFailure).toBe(true); + + // Server should still be running (load() didn't throw) + // Country and lang register as factories, not providers + expect(library.codeSystemFactories.size).toBeGreaterThanOrEqual(2); + }, 30000); + + test('failed source is skipped in subsequent loading phases', async () => { + await fs.writeFile(yamlPath, [ + 'base:', + ' url: https://storage.googleapis.com/tx-fhir-org', + 'sources:', + ' - internal:lang', + ' - npm:nonexistent.package.that.does.not.exist#99.99.99', + ' - internal:country', + ].join('\n')); + + const { library, log } = createLibrary(yamlPath); + + // Spy on processSource to track calls + const originalProcessSource = library.processSource.bind(library); + const calls = []; + library.processSource = async function (source, pm, mode) { + calls.push({ source, mode }); + return originalProcessSource(source, pm, mode); + }; + + await library.load(); + + // The npm source should have been called in fetch phase but NOT in cs/npm phases + const npmFetchCalls = calls.filter(c => c.source.includes('nonexistent') && c.mode === 'fetch'); + const npmCsCalls = calls.filter(c => c.source.includes('nonexistent') && c.mode === 'cs'); + const npmNpmCalls = calls.filter(c => c.source.includes('nonexistent') && c.mode === 'npm'); + + expect(npmFetchCalls.length).toBe(1); + expect(npmCsCalls.length).toBe(0); + expect(npmNpmCalls.length).toBe(0); + }, 60000); + + test('summary warning lists all failed sources', async () => { + await fs.writeFile(yamlPath, [ + 'base:', + ' url: https://storage.googleapis.com/tx-fhir-org', + 'sources:', + ' - internal:BOGUS_ONE', + ' - internal:lang', + ' - internal:BOGUS_TWO', + ].join('\n')); + + const { library, log } = createLibrary(yamlPath); + await library.load(); + + const warnCalls = log.warn.mock.calls.map(c => c[0]); + const summary = warnCalls.find(msg => msg.includes('source(s) failed to load')); + expect(summary).toBeDefined(); + expect(summary).toContain('BOGUS_ONE'); + expect(summary).toContain('BOGUS_TWO'); + expect(summary).toMatch(/^2 source/); + }, 30000); + + test('load succeeds with empty sources', async () => { + await fs.writeFile(yamlPath, [ + 'base:', + ' url: https://storage.googleapis.com/tx-fhir-org', + 'sources: []', + ].join('\n')); + + const { library, log } = createLibrary(yamlPath); + await library.load(); + + // No errors, no warnings + expect(log.error).not.toHaveBeenCalled(); + expect(log.warn).not.toHaveBeenCalled(); + }, 30000); +}); diff --git a/tx/library.js b/tx/library.js index c1cada9..cfc55f0 100644 --- a/tx/library.js +++ b/tx/library.js @@ -157,8 +157,16 @@ class Library { this.log.info('Fetching Data from '+this.baseUrl); + const failedSources = []; for (const source of config.sources) { - await this.processSource(source, this.packageManager, "fetch"); + try { + await this.processSource(source, this.packageManager, "fetch"); + } catch (error) { + const msg = `Failed to fetch source '${source}': ${error.message}`; + console.error(msg); + this.log.error(msg); + failedSources.push(source); + } } this.log.info("Downloaded "+((this.totalDownloaded + this.packageManager.totalDownloaded)/ 1024)+" kB"); @@ -167,13 +175,34 @@ class Library { this.#logSystemHeader(); for (const source of config.sources) { - await this.processSource(source, this.packageManager, "cs"); + if (failedSources.includes(source)) continue; + try { + await this.processSource(source, this.packageManager, "cs"); + } catch (error) { + const msg = `Failed to load code systems from '${source}': ${error.message}`; + console.error(msg); + this.log.error(msg); + failedSources.push(source); + } } this.log.info('Loading Packages'); this.#logPackagesHeader(); for (const source of config.sources) { - await this.processSource(source, this.packageManager, "npm"); + if (failedSources.includes(source)) continue; + try { + await this.processSource(source, this.packageManager, "npm"); + } catch (error) { + const msg = `Failed to load package '${source}': ${error.message}`; + console.error(msg); + this.log.error(msg); + } + } + + if (failedSources.length > 0) { + const msg = `${failedSources.length} source(s) failed to load: ${failedSources.join(', ')}`; + console.warn(msg); + this.log.warn(msg); } const endMemory = process.memoryUsage(); diff --git a/tx/library/codesystem.js b/tx/library/codesystem.js index da3f1ad..872359f 100644 --- a/tx/library/codesystem.js +++ b/tx/library/codesystem.js @@ -33,7 +33,12 @@ class CodeSystem extends CanonicalResource { // Convert to R5 format internally (modifies input for performance) this.jsonObj = codeSystemToR5(this.jsonObj, fhirVersion); if (!noMaps) { - this.validate(); + try { + this.validate(); + } catch (e) { + const id = this.jsonObj?.url ? `${this.jsonObj.url}|${this.jsonObj.version || ''}` : this.jsonObj?.name || 'unknown'; + throw new Error(`${e.message} (in ${id})`); + } this.buildMaps(); } } From 183d933e662a20f2b748efc75c4610b90c78ad86 Mon Sep 17 00:00:00 2001 From: Jose Costa Teixeira <16153168+costateixeira@users.noreply.github.com> Date: Thu, 19 Feb 2026 18:34:00 +0100 Subject: [PATCH 2/2] Improve error reporting for source loading failures Errors during source loading now print to console before failing, so they are visible in Docker output (Winston suppresses errors from console by default). The error message identifies which source failed. Co-Authored-By: Claude --- library/package-manager.js | 69 +++++++++-------- tests/tx/library-error-handling.test.js | 98 +++++++++++++++++++++++++ tx/library.js | 21 +++++- tx/library/codesystem.js | 7 +- 4 files changed, 159 insertions(+), 36 deletions(-) create mode 100644 tests/tx/library-error-handling.test.js diff --git a/library/package-manager.js b/library/package-manager.js index 00693d1..5ac4fc7 100644 --- a/library/package-manager.js +++ b/library/package-manager.js @@ -594,41 +594,46 @@ class PackageManager { */ async fetchUrl(url) { console.log("Fetch Package from URL: " + url); - const client = new CIBuildClient(); - const packageData = await client.fetchFromUrlSpecific(url); - - // Extract to a temp location to read package.json for name and version - const tempKey = `_url_temp_${Date.now()}`; - const tempPath = await this.extractToCache(tempKey, 'url', packageData); - const tempFullPath = path.join(this.cacheFolder, tempPath); - - // Read package name and version from the extracted package - const pkgJsonPath = path.join(tempFullPath, 'package', 'package.json'); - const pkgJson = JSON.parse(await fs.readFile(pkgJsonPath, 'utf8')); - const packageId = pkgJson.name; - const version = pkgJson.version; - - if (!packageId || !version) { - throw new Error(`Package at ${url} has no name or version in package.json`); - } + try { + const client = new CIBuildClient(); + const packageData = await client.fetchFromUrlSpecific(url); + + // Extract to a temp location to read package.json for name and version + const tempKey = `_url_temp_${Date.now()}`; + const tempPath = await this.extractToCache(tempKey, 'url', packageData); + const tempFullPath = path.join(this.cacheFolder, tempPath); + + // Read package name and version from the extracted package + const pkgJsonPath = path.join(tempFullPath, 'package', 'package.json'); + const pkgJson = JSON.parse(await fs.readFile(pkgJsonPath, 'utf8')); + const packageId = pkgJson.name; + const version = pkgJson.version; + + if (!packageId || !version) { + throw new Error(`Package at ${url} has no name or version in package.json`); + } - // Use the same cache key format as npm packages - const finalName = `${packageId}#${version}`; - const finalPath = path.join(this.cacheFolder, finalName); + // Use the same cache key format as npm packages + const finalName = `${packageId}#${version}`; + const finalPath = path.join(this.cacheFolder, finalName); - // If it already exists, the same package is already loaded - that's a config error - try { - await fs.access(finalPath); - await fs.rm(tempFullPath, { recursive: true, force: true }); - throw new Error(`Package ${finalName} already exists in cache. Check library config for duplicates (url: ${url})`); - } catch (e) { - if (e.message.includes('already exists')) throw e; - // Doesn't exist yet, rename temp to final - await fs.rename(tempFullPath, finalPath); - } + // If it already exists, the same package is already loaded - that's a config error + try { + await fs.access(finalPath); + await fs.rm(tempFullPath, { recursive: true, force: true }); + throw new Error(`Package ${finalName} already exists in cache. Check library config for duplicates (url: ${url})`); + } catch (e) { + if (e.message.includes('already exists')) throw e; + // Doesn't exist yet, rename temp to final + await fs.rename(tempFullPath, finalPath); + } - this.totalDownloaded = this.totalDownloaded + packageData.length; - return finalName; + this.totalDownloaded = this.totalDownloaded + packageData.length; + return finalName; + } catch (error) { + console.error(`Error fetching package from URL ${url}: ${error.message}`); + throw error; + } } /** diff --git a/tests/tx/library-error-handling.test.js b/tests/tx/library-error-handling.test.js new file mode 100644 index 0000000..af8f17b --- /dev/null +++ b/tests/tx/library-error-handling.test.js @@ -0,0 +1,98 @@ +const { Library } = require('../../tx/library'); +const path = require('path'); +const fs = require('fs').promises; +const os = require('os'); + +describe('Library error handling', () => { + let tmpDir; + let yamlPath; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'lib-test-')); + yamlPath = path.join(tmpDir, 'library.yml'); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + function createLibrary(configFile) { + const log = { + info: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + debug: jest.fn(), + }; + const stats = { addStat: jest.fn() }; + return { library: new Library(configFile, undefined, log, stats), log }; + } + + test('failed source reports which source failed and throws', async () => { + await fs.writeFile(yamlPath, [ + 'base:', + ' url: https://storage.googleapis.com/tx-fhir-org', + 'sources:', + ' - internal:lang', + ' - internal:INVALID_SOURCE', + ' - internal:country', + ].join('\n')); + + const { library } = createLibrary(yamlPath); + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + + await expect(library.load()).rejects.toThrow(); + + // The error message should identify the failing source + const errorMessages = consoleSpy.mock.calls.map(c => c[0]); + expect(errorMessages.some(msg => msg.includes('INVALID_SOURCE'))).toBe(true); + + consoleSpy.mockRestore(); + }, 30000); + + test('error message includes source name on fetch failure', async () => { + await fs.writeFile(yamlPath, [ + 'base:', + ' url: https://storage.googleapis.com/tx-fhir-org', + 'sources:', + ' - npm:nonexistent.package.that.does.not.exist#99.99.99', + ].join('\n')); + + const { library } = createLibrary(yamlPath); + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + + await expect(library.load()).rejects.toThrow(); + + const errorMessages = consoleSpy.mock.calls.map(c => c[0]); + expect(errorMessages.some(msg => msg.includes('nonexistent.package'))).toBe(true); + + consoleSpy.mockRestore(); + }, 60000); + + test('load succeeds with empty sources', async () => { + await fs.writeFile(yamlPath, [ + 'base:', + ' url: https://storage.googleapis.com/tx-fhir-org', + 'sources: []', + ].join('\n')); + + const { library, log } = createLibrary(yamlPath); + await library.load(); + + expect(log.error).not.toHaveBeenCalled(); + }, 30000); + + test('load succeeds with valid sources', async () => { + await fs.writeFile(yamlPath, [ + 'base:', + ' url: https://storage.googleapis.com/tx-fhir-org', + 'sources:', + ' - internal:lang', + ' - internal:country', + ].join('\n')); + + const { library } = createLibrary(yamlPath); + await library.load(); + + expect(library.codeSystemFactories.size).toBeGreaterThanOrEqual(2); + }, 30000); +}); diff --git a/tx/library.js b/tx/library.js index c1cada9..ddfe622 100644 --- a/tx/library.js +++ b/tx/library.js @@ -158,7 +158,12 @@ class Library { this.log.info('Fetching Data from '+this.baseUrl); for (const source of config.sources) { - await this.processSource(source, this.packageManager, "fetch"); + try { + await this.processSource(source, this.packageManager, "fetch"); + } catch (error) { + console.error(`Failed to fetch source '${source}': ${error.message}`); + throw error; + } } this.log.info("Downloaded "+((this.totalDownloaded + this.packageManager.totalDownloaded)/ 1024)+" kB"); @@ -167,13 +172,23 @@ class Library { this.#logSystemHeader(); for (const source of config.sources) { - await this.processSource(source, this.packageManager, "cs"); + try { + await this.processSource(source, this.packageManager, "cs"); + } catch (error) { + console.error(`Failed to load code systems from '${source}': ${error.message}`); + throw error; + } } this.log.info('Loading Packages'); this.#logPackagesHeader(); for (const source of config.sources) { - await this.processSource(source, this.packageManager, "npm"); + try { + await this.processSource(source, this.packageManager, "npm"); + } catch (error) { + console.error(`Failed to load package '${source}': ${error.message}`); + throw error; + } } const endMemory = process.memoryUsage(); diff --git a/tx/library/codesystem.js b/tx/library/codesystem.js index da3f1ad..872359f 100644 --- a/tx/library/codesystem.js +++ b/tx/library/codesystem.js @@ -33,7 +33,12 @@ class CodeSystem extends CanonicalResource { // Convert to R5 format internally (modifies input for performance) this.jsonObj = codeSystemToR5(this.jsonObj, fhirVersion); if (!noMaps) { - this.validate(); + try { + this.validate(); + } catch (e) { + const id = this.jsonObj?.url ? `${this.jsonObj.url}|${this.jsonObj.version || ''}` : this.jsonObj?.name || 'unknown'; + throw new Error(`${e.message} (in ${id})`); + } this.buildMaps(); } }