diff --git a/package-lock.json b/package-lock.json index c977943e8..a3eaf1562 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,5 +1,5 @@ { - "name": "skip", + "name": "work", "lockfileVersion": 3, "requires": true, "packages": { diff --git a/skipruntime-ts/tests/BRIDGE_EDGE_CASES.md b/skipruntime-ts/tests/BRIDGE_EDGE_CASES.md new file mode 100644 index 000000000..cf5eb859a --- /dev/null +++ b/skipruntime-ts/tests/BRIDGE_EDGE_CASES.md @@ -0,0 +1,70 @@ +**JS↔Skip Bridge Edge Cases** + +This document describes edge case tests added to verify the behavior of the JavaScript↔Skip bridge. These tests document current behavior, some of which is problematic. + +## Overview + +The Skip Runtime has two implementations: +- **WASM**: WebAssembly-based (`@skipruntime/wasm`) +- **Native**: Node.js C++ addon (`@skipruntime/native`) + +Both share the same TypeScript interface and JSON serialization layer. The tests below verify that both implementations handle JavaScript edge cases consistently. + +--- + +## Test: `testNaNHandling` + +**What it tests**: How `NaN` (Not a Number) is handled when stored in collections. + +**Current behavior**: NaN passes through unchanged. + +**Why this is problematic**: +- `NaN` is **not a valid JSON value** per RFC 8259 +- `JSON.stringify(NaN)` returns `"null"`, not `"NaN"` +- Passing NaN through could cause issues when data is serialized/deserialized +- Different systems may handle NaN inconsistently + +**Recommendation**: Should throw an error: "Cannot export NaN: not a valid JSON value" + +--- + +## Test: `testInfinityHandling` + +**What it tests**: How `Infinity` and `-Infinity` are handled when stored in collections. + +**Current behavior**: 🔴 **SILENT DATA CORRUPTION** +``` +Infinity → 9223372036854776000 (≈ 2^63) +-Infinity → -9223372036854776000 +``` + +**Why this is problematic**: +- `Infinity` is **not a valid JSON value** per RFC 8259 +- The current code path causes integer overflow: + ```typescript + // In exportJSON(): + if (value === Math.trunc(value)) { // Math.trunc(Infinity) === Infinity → true! + return binding.SKIP_SKJSON_createCJInt(value); // Overflows! + } + ``` +- Users store `Infinity`, get back a huge but finite number - **data corruption** +- No error is thrown, making this very hard to debug + +**Recommendation**: Should throw an error: "Cannot export Infinity: not a valid JSON value" + +--- + +## Test: `testLargeIntegerPrecision` + +**What it tests**: How integers larger than `Number.MAX_SAFE_INTEGER` (2^53 - 1) are handled. + +**Current behavior**: Large integers are preserved through the Skip runtime. + +**Why this could be problematic**: +- JavaScript numbers lose precision beyond 2^53 +- The value `9007199254740993` cannot be exactly represented in JS +- Skip uses 64-bit integers internally, which can represent larger values +- Round-tripping through JS could lose precision for values > 2^53 + +**Note**: This is a fundamental JavaScript limitation, not a Skip bug. Users working with large integers should be aware of this. + diff --git a/skipruntime-ts/tests/src/tests.ts b/skipruntime-ts/tests/src/tests.ts index f797c8513..e14be0360 100644 --- a/skipruntime-ts/tests/src/tests.ts +++ b/skipruntime-ts/tests/src/tests.ts @@ -579,6 +579,32 @@ const booleanRoundtripService: SkipService = { }, }; +//// Edge case tests for JS↔Skip bridge + +// Identity mapper that preserves JSON values through roundtrip +class IdentityMapper implements Mapper { + mapEntry(key: Json, values: Values): Iterable<[Json, Json]> { + return values.toArray().map((v) => [key, v]); + } +} + +type Input_JJ = { input: EagerCollection }; + +class IdentityResource implements Resource { + instantiate(cs: Input_JJ): EagerCollection { + return cs.input.map(IdentityMapper); + } +} + +const identityService: SkipService = { + initialData: { input: [] }, + resources: { identity: IdentityResource }, + + createGraph(inputCollections: Input_JJ) { + return inputCollections; + }, +}; + //// testExternalService async function timeout(ms: number) { @@ -1697,6 +1723,118 @@ export function initTests( await service.close(); }); + // Edge case tests for JS↔Skip bridge + // These tests verify correct handling of JavaScript edge cases + // See BRIDGE_EDGE_CASES.md for documentation of what each test verifies + + it("testNaNHandling", async () => { + // NaN is not a valid JSON value - ideally should be rejected + // Currently it passes through as NaN (WASM) which is questionable + const service = await initService(identityService); + try { + const resource = "identity"; + try { + await service.update("input", [["nan_key", [NaN]]]); + const result = await service.getAll(resource); + // If we get here, NaN was accepted - check what it became + const nanVal = (result.find((e) => e[0] === "nan_key") as Entry< + Json, + Json + >)[1][0]; + console.log("NaN became:", nanVal, "type:", typeof nanVal); + // Document current behavior: + // - WASM: NaN passes through as NaN (questionable - not valid JSON) + // - Native: may differ + // Ideally this should throw an error since NaN is not valid JSON + expect(typeof nanVal).toEqual("number"); + } catch (e) { + // This is the PREFERRED behavior - rejecting NaN + expect(e).toBeA(Error); + } + } finally { + await service.close(); + } + }); + + it("testInfinityHandling", async () => { + // BUG: Infinity is not a valid JSON value but is silently corrupted! + // Math.trunc(Infinity) === Infinity, so it goes to createCJInt which overflows + const service = await initService(identityService); + try { + const resource = "identity"; + try { + await service.update("input", [ + ["pos_inf", [Infinity]], + ["neg_inf", [-Infinity]], + ]); + const result = await service.getAll(resource); + // If we get here, Infinity was accepted - check what it became + const posInf = (result.find((e) => e[0] === "pos_inf") as Entry< + Json, + Json + >)[1][0] as number; + const negInf = (result.find((e) => e[0] === "neg_inf") as Entry< + Json, + Json + >)[1][0] as number; + console.log("Infinity became:", posInf, "type:", typeof posInf); + console.log("-Infinity became:", negInf, "type:", typeof negInf); + // BUG: Currently Infinity silently overflows to a huge number (~2^63) + // This is silent data corruption! Should throw instead. + // The test documents the bug - Infinity should NOT become a finite number + if (Number.isFinite(posInf)) { + // This is the bug - we're getting a corrupted value + console.error( + "BUG: Infinity was silently corrupted to finite number:", + posInf, + ); + } + // For now, just verify we don't crash - the real fix should throw + expect(typeof posInf).toEqual("number"); + expect(typeof negInf).toEqual("number"); + } catch (e) { + // This SHOULD happen - rejecting Infinity is correct behavior + expect(e).toBeA(Error); + } + } finally { + await service.close(); + } + }); + + it("testLargeIntegerPrecision", async () => { + // Numbers larger than Number.MAX_SAFE_INTEGER may lose precision + const service = await initService(identityService); + try { + const resource = "identity"; + const maxSafeInt = Number.MAX_SAFE_INTEGER; // 2^53 - 1 = 9007199254740991 + const beyondSafe = maxSafeInt + 2; // 9007199254740993 - not representable exactly + await service.update("input", [ + ["max_safe", [maxSafeInt]], + ["beyond_safe", [beyondSafe]], + ]); + const result = await service.getAll(resource); + const maxSafeResult = (result.find((e) => e[0] === "max_safe") as Entry< + Json, + Json + >)[1][0] as number; + const beyondSafeResult = ( + result.find((e) => e[0] === "beyond_safe") as Entry + )[1][0] as number; + // MAX_SAFE_INTEGER should be preserved exactly + expect(maxSafeResult).toEqual(maxSafeInt); + // Beyond safe integer - document the behavior (may lose precision) + console.log( + "Beyond safe integer:", + beyondSafe, + "became:", + beyondSafeResult, + ); + // This test documents the limitation - ideally we'd validate or warn + } finally { + await service.close(); + } + }); + it("testExternal", async () => { const serviceDef = testExternalService(); const mockExternal = serviceDef.externalServices![