JavaScript SDK: fix config read errors silently degrading to an empty config (KSM-1266)

[stas-schaller]

Summary

JavaScript SDK: fixes localConfigStorage treating every config-read failure as "no config yet."

Changes

Fixed

• readStorage caught every exception from reading or parsing the config file and returned an empty config, masking permission errors and malformed JSON as a fresh start. Only a missing file (ENOENT) is treated that way now; everything else throws a KeeperError. (KSM-1266)

Testing

cd sdk/javascript/packages/core && npm test -- test/localConfigStorage.test.ts

Breaking Changes

None as a public API change, but a behavior change: localConfigStorage(configName) can now throw KeeperError for a config file that exists but is unreadable or malformed, where it previously started fresh silently. Callers that only expected the missing-file case are unaffected.

Related Issues

• Jira: KSM-1266

[mgallego-keeper]

Review summary

Correct, well-targeted fix for KSM-1266. JSON.parse's SyntaxError has no .code property, so malformed JSON correctly falls through to the KeeperError branch rather than being mistaken for ENOENT; verified this holds. The three new tests (missing file, unreadable file, malformed JSON) match the intended behavior exactly, and the PR body's own disclosure of the behavior change (localConfigStorage can now throw where it previously started fresh silently) is accurate and, in my view, the right tradeoff: masking a permission error or corrupt config as "no config yet" is worse than a loud failure. One robustness note on the new tests below, not blocking.

Notes

The "unreadable config file" test assumes a non-root process

chmodSync(configPath, 0o000) only blocks read access for a non-root user. If this suite is ever run as root (a common default in Docker-based Node images or some CI containers), fs.readFileSync will still succeed despite mode 0, the file will parse as valid empty JSON, and readStorage won't throw at all, so this test would fail to exercise the code path it's meant to cover (and would fail outright, since the toThrow(KeeperError) assertion would not be satisfied). Not a defect in the production code, just a latent assumption in this specific test. Worth a comment noting the non-root assumption, or skipping the test when process.getuid?.() === 0, so a future root-run CI environment doesn't quietly lose this coverage.

Minor style nit

catch (e: Error | any) is unusual TypeScript; Error | any collapses to plain any during type resolution (confirmed this compiles cleanly under strict: true), so it reads as more specific than it actually is. Purely cosmetic, no functional difference from catch (e: any).