[SELF-662] feat: add mobile sdk error classes #916
Conversation
|
Warning Rate limit exceeded@transphorm has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds typed error classes across mobile and core SDKs and replaces generic throws in MRZ/NFC processing and verifier paths. Updates exports to surface new errors, adds docs describing error taxonomy, and introduces tests validating error types. Adjusts core test runner to ts-node. No business logic changes beyond error typing. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Code
participant MRZ as MRZ Processor
participant NFC as NFC Parser
App->>MRZ: extractMRZInfo(input)
MRZ-->>App: return data
MRZ-x App: throw MrzParseError (invalid/malformed)
note over MRZ,App: Typed error exposes category=validation, code=SELF_ERR_MRZ_PARSE
App->>NFC: parseNFCResponse(bytes)
NFC-->>App: return TLVs
NFC-x App: throw NfcParseError (truncated/unsupported)
note over NFC,App: Typed error exposes category=validation, code=SELF_ERR_NFC_PARSE
sequenceDiagram
participant Client as Core Client
participant Verifier as SelfBackendVerifier
participant Registry as Registry Lookup
Client->>Verifier: verify(...)
Verifier->>Registry: getVerifierAddress()
alt Found
Registry-->>Verifier: address
Verifier-->>Client: proceed/return
else Not found / failure
Verifier-x Client: throw RegistryContractError / VerifierContractError
note over Verifier,Client: Domain-specific errors replace generic Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Giga Summary• feat: add typed errors
Quality Assessment• Strengths: Improved error handling, better documentation, enhanced testing. Quality Score: 8/10 (Threshold: 7/10) 💬 Detailed comments have been added to specific code changes. |
![giga-agent[bot]](https://avatars.githubusercontent.com/in/1202121?s=60&v=4){kind=small}
transphorm
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/mobile-sdk-alpha/src/processing/nfc.ts (1)
96-111: Prevent untyped exceptions leaking from TextDecoder.decodegetDecoder().decode(value) with fatal: true can throw a DOMException/TypeError on invalid UTF‑8, which bypasses your typed error surface. Wrap and rethrow as NfcParseError with cause.
switch (tag) { case TAG_DG1: { - result.dg1 = { mrz: getDecoder().decode(value) }; + try { + const mrz = getDecoder().decode(value); + result.dg1 = { mrz }; + } catch (err) { + throw new NfcParseError('Invalid UTF-8 in DG1 value', { cause: err }); + } break; }packages/mobile-sdk-alpha/src/errors/SdkError.ts (1)
23-33: Unsafe cast of cause may crash at runtime — coerce or wrap non-Error causesCasting unknown to Error is unsound; if a non-Error (e.g., string, Axios response) is passed, downstream code may dereference properties that don't exist. Ensure cause is an Error instance or wrap it.
Apply this diff to make cause handling robust:
export class SdkError extends Error { readonly code: string; readonly category: SdkErrorCategory; readonly retryable: boolean; declare cause?: Error; constructor(message: string, code: string, category: SdkErrorCategory, retryable = false, options?: ErrorOptions) { super(message); this.name = 'SdkError'; this.code = code; this.category = category; this.retryable = retryable; - if (options?.cause) { - this.cause = options.cause as Error; - } + if (options && 'cause' in options && options.cause !== undefined) { + this.cause = options.cause instanceof Error + ? options.cause + : new Error(String(options.cause)); + } } }
♻️ Duplicate comments (8)
sdk/core/package.json (1)
45-45: Harden test discovery to avoid silently skipping testsThe current pattern runs only direct children of tests/.test.ts. If new tests land under nested folders (e.g., tests/unit/.test.ts) they will be skipped, leading to false greens. Also, relying on shell globbing can be inconsistent across platforms.
Apply this diff to make discovery robust:
- "test": "node --loader ts-node/esm --test tests/*.test.ts", + "test": "node --loader ts-node/esm --test \"tests/**/*.test.ts\"",Run this to list any tests that your current script would miss (anything not in tests/*.test.ts):
#!/bin/bash # Show all test files, then filter those not directly under tests/ echo "All .test.ts files under sdk/core/tests:" fd -t f -e ts -g "sdk/core/tests/**" | rg -nP '\.test\.ts$' -n echo "" echo "Potentially skipped (nested) tests relative to sdk/core/tests:" fd -t f -e ts -g "sdk/core/tests/**" | rg -nP '\.test\.ts$' -n | awk -F'/tests/' '{print $2}' | rg -nP '.+/.+\.test\.ts$' || truesdk/core/tests/verifier-errors.test.ts (1)
5-13: Strengthen tests: assert instanceof and message, not just the nameAsserting only on
err.nameis brittle. Validate the constructor type and the message to ensure correct error wiring and future-proof against regressions.Apply this diff:
test('creates registry contract error', () => { const err = new RegistryContractError('Registry contract not found'); - assert.equal(err.name, 'RegistryContractError'); + assert.ok(err instanceof RegistryContractError); + assert.equal(err.message, 'Registry contract not found'); }); test('creates verifier contract error', () => { const err = new VerifierContractError('Verifier contract not found'); - assert.equal(err.name, 'VerifierContractError'); + assert.ok(err instanceof VerifierContractError); + assert.equal(err.message, 'Verifier contract not found'); });sdk/core/src/utils/proof.ts (1)
4-4: Good switch to typed ProofError; add code/details for programmatic handlingTyped error is a solid improvement. To make this actionable for SDK consumers, add a stable
codeand structured details (e.g., the offendingattestationId).Apply this diff to include structured context at the throw site:
- throw new ProofError(`Invalid attestation ID: ${attestationId}`); + throw new ProofError(`Invalid attestation ID: ${attestationId}`, { + // enables programmatic error handling + code: 'INVALID_ATTESTATION_ID', + attestationId, + } as any);And update the error class to support code/details (in sdk/core/src/errors/ProofError.ts):
export class ProofError extends Error { - constructor(message: string) { - super(message); - this.name = 'ProofError'; - } + public readonly code?: string; + public readonly attestationId?: number; + constructor( + message: string, + options?: (ErrorOptions & { code?: string; attestationId?: number }) + ) { + super(message, options); + this.name = 'ProofError'; + this.code = options?.code; + this.attestationId = options?.attestationId; + Object.setPrototypeOf(this, new.target.prototype); + if (Error.captureStackTrace) Error.captureStackTrace(this, ProofError); + } }Also applies to: 21-21
packages/mobile-sdk-alpha/tests/processing/mrz.test.ts (1)
15-18: Asserting specific error type improves test precisionSwitching to toThrowError(MrzParseError) tightens the contract around failure modes.
packages/mobile-sdk-alpha/src/errors/InitError.ts (1)
9-14: Consider subcodes for common init failure modesEchoing prior feedback: using more specific codes (e.g., SELF_ERR_INIT_CONFIG, SELF_ERR_INIT_ENV, SELF_ERR_INIT_NETWORK) improves observability and automated handling.
packages/mobile-sdk-alpha/src/errors/LivenessError.ts (1)
9-14: Consider more granular liveness error codesAs with InitError, subcodes (e.g., SELF_ERR_LIVENESS_SPOOF, SELF_ERR_LIVENESS_TIMEOUT) can help clients tailor UX and retries.
packages/mobile-sdk-alpha/src/errors/MrzParseError.ts (1)
11-11: Centralize the MRZ error code to avoid magic stringsTo prevent typos and keep codes consistent across packages, centralize 'SELF_ERR_MRZ_PARSE' in a shared constants module (similar to SCANNER_ERROR_CODES) and reference it here. This also eases future refactors and documentation alignment.
packages/mobile-sdk-alpha/src/errors/index.ts (1)
12-12: Deprecate the generic sdkError to enforce the new typed error taxonomyContinuing to expose a generic factory encourages bypassing domain-specific errors, making handling less precise. Consider marking sdkError as deprecated in SdkError.ts to guide consumers toward InitError, LivenessError, MrzParseError, and NfcParseError.
Apply this diff in packages/mobile-sdk-alpha/src/errors/SdkError.ts to deprecate the helper:
/** * Convenience factory for {@link SdkError}. + * + * @deprecated Prefer domain-specific error classes (e.g., InitError, LivenessError, MrzParseError, NfcParseError) + * to preserve error taxonomy and enable targeted handling. This factory will be removed in a future release. * * @param message - error description. * @param code - unique error code. * @param category - high level error category. * @param retryable - whether the operation may be retried. * @returns configured {@link SdkError} instance. */ export function sdkError(message: string, code: string, category: SdkErrorCategory, retryable = false) { return new SdkError(message, code, category, retryable); }Additionally, verify that legacy throw new Error(...) and sdkError(...) usages in the alpha package have been replaced by typed errors:
#!/bin/bash set -euo pipefail echo "Scanning for generic Error throws in mobile-sdk-alpha (excluding tests)..." rg -nP --type=ts --type=tsx -g 'packages/mobile-sdk-alpha/**' -g '!packages/mobile-sdk-alpha/tests/**' -g '!**/__tests__/**' -C2 '\bthrow\s+new\s+Error\s*\(' || true echo echo "Scanning for sdkError() factory usage in mobile-sdk-alpha..." rg -nP --type=ts --type=tsx -g 'packages/mobile-sdk-alpha/**' -C2 '\bsdkError\s*\(' || true
🧹 Nitpick comments (11)
sdk/core/src/errors/VerifierContractError.ts (1)
1-11: Add cause + contract context to improve debuggability and programmatic handlingFor smart-contract failures in mobile/RN environments, preserving the underlying cause and minimal on-chain context (chainId, contract address, method) materially improves error triage and telemetry without leaking secrets. Also, setting the prototype explicitly keeps instanceof checks reliable across different runtimes/bundlers.
Apply this diff to extend the error with optional context while keeping backward compatibility:
export class VerifierContractError extends Error { - constructor(message: string) { - super(message); - this.name = 'VerifierContractError'; - } + readonly chainId?: number | string; + readonly contractAddress?: string; + readonly method?: string; + readonly code?: string; + // Note: keep second arg optional to avoid breaking existing call sites. + constructor( + message: string, + opts: { + cause?: unknown; + chainId?: number | string; + contractAddress?: string; + method?: string; + code?: string; // e.g., 'CONTRACT_NOT_FOUND' | 'CALL_REVERTED' + } = {}, + ) { + super(message); + this.name = 'VerifierContractError'; + // Preserve prototype for reliable instanceof in diverse JS engines/bundlers. + Object.setPrototypeOf(this, new.target.prototype); + // Attach context (safe, non-PII) + this.chainId = opts.chainId; + this.contractAddress = opts.contractAddress; + this.method = opts.method; + this.code = opts.code; + // Preserve root cause (Node 16+/Hermes support reading error.cause as any) + if (opts.cause !== undefined) { + (this as any).cause = opts.cause; + } + } }sdk/core/src/errors/RegistryContractError.ts (1)
1-11: Include root cause and registry context (chainId/address) for actionable errorsSurface-level messages are often insufficient when registry lookups fail across networks. Add optional metadata and cause to enable better telemetry, retries, and user messaging without exposing PII.
Apply this diff:
export class RegistryContractError extends Error { - constructor(message: string) { - super(message); - this.name = 'RegistryContractError'; - } + readonly chainId?: number | string; + readonly contractAddress?: string; + readonly method?: string; + readonly code?: string; + constructor( + message: string, + opts: { + cause?: unknown; + chainId?: number | string; + contractAddress?: string; + method?: string; + code?: string; // e.g., 'REGISTRY_NOT_FOUND' | 'CALL_FAILED' + } = {}, + ) { + super(message); + this.name = 'RegistryContractError'; + Object.setPrototypeOf(this, new.target.prototype); + this.chainId = opts.chainId; + this.contractAddress = opts.contractAddress; + this.method = opts.method; + this.code = opts.code; + if (opts.cause !== undefined) { + (this as any).cause = opts.cause; + } + } }sdk/core/tests/proof.test.ts (1)
6-8: Add positive-path assertions to prevent regressions in computed lengthsOnly the invalid-path is covered. Adding assertions for known valid attestation IDs (1 and 2) catches accidental changes in constants/rounding that would break proofs at runtime.
You can append tests like:
import test from 'node:test'; import assert from 'node:assert'; import { getRevealedDataPublicSignalsLength } from '../src/utils/proof.js'; test('attestation id 1 -> exact 3 signals', () => { assert.strictEqual(getRevealedDataPublicSignalsLength(1 as any), 3); }); test('attestation id 2 -> ceil(94/31)=4 signals', () => { assert.strictEqual(getRevealedDataPublicSignalsLength(2 as any), 4); });sdk/core/src/errors/ProofError.ts (1)
1-11: Preserve error cause to aid diagnosis of proof failuresProof generation/verification can fail deep in crypto libs; threading the root cause up the stack significantly reduces MTTR. Explicitly set prototype to keep instanceof stable across transpilers and RN.
Apply this diff:
export class ProofError extends Error { - constructor(message: string) { - super(message); - this.name = 'ProofError'; - } + readonly code?: string; // e.g., 'INVALID_ATTESTATION_ID' | 'WITNESS_GENERATION_FAILED' + constructor(message: string, opts: { cause?: unknown; code?: string } = {}) { + super(message); + this.name = 'ProofError'; + Object.setPrototypeOf(this, new.target.prototype); + this.code = opts.code; + if (opts.cause !== undefined) { + (this as any).cause = opts.cause; + } + } }sdk/core/src/errors/index.ts (1)
31-33: Ensure custom errors preserve prototype and support error causesFor reliable
instanceofchecks across TS transpilation targets and better observability, customize constructors to set the prototype and acceptErrorOptions(cause). This becomes more important now that errors are part of the public API and used in control flow.Suggested updates:
sdk/core/src/errors/RegistryContractError.ts
export class RegistryContractError extends Error { - constructor(message: string) { - super(message); - this.name = 'RegistryContractError'; - } + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = 'RegistryContractError'; + Object.setPrototypeOf(this, new.target.prototype); + if (Error.captureStackTrace) Error.captureStackTrace(this, RegistryContractError); + } }sdk/core/src/errors/VerifierContractError.ts
export class VerifierContractError extends Error { - constructor(message: string) { - super(message); - this.name = 'VerifierContractError'; - } + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = 'VerifierContractError'; + Object.setPrototypeOf(this, new.target.prototype); + if (Error.captureStackTrace) Error.captureStackTrace(this, VerifierContractError); + } }sdk/core/src/errors/ProofError.ts (if not already updated as per proof.ts comment)
export class ProofError extends Error { - constructor(message: string) { - super(message); - this.name = 'ProofError'; - } + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = 'ProofError'; + Object.setPrototypeOf(this, new.target.prototype); + if (Error.captureStackTrace) Error.captureStackTrace(this, ProofError); + } }sdk/core/src/SelfBackendVerifier.ts (1)
121-121: Preserve underlying causes and add context to contract-not-found errorsCurrent rethrows discard the original error and lack context, making production incidents hard to diagnose (network/rpc failures vs. actual “not found”). Include
causeand relevant identifiers in the message.Apply this diff:
@@ - if (registryAddress === '0x0000000000000000000000000000000000000000') { - throw new RegistryContractError('Registry contract not found'); + if (registryAddress === '0x0000000000000000000000000000000000000000') { + throw new RegistryContractError( + `Registry contract not found for attestationId ${attestationId}` + ); } @@ - } catch (error) { - throw new RegistryContractError('Registry contract not found'); + } catch (error) { + throw new RegistryContractError('Failed to resolve registry contract', { + cause: error as Error, + }); } @@ - if (verifierAddress === '0x0000000000000000000000000000000000000000') { - throw new VerifierContractError('Verifier contract not found'); + if (verifierAddress === '0x0000000000000000000000000000000000000000') { + throw new VerifierContractError( + `Verifier contract not found for attestationId ${attestationId}` + ); } @@ - } catch (error) { - throw new VerifierContractError('Verifier contract not found'); + } catch (error) { + throw new VerifierContractError('Failed to resolve verifier contract', { + cause: error as Error, + }); }Also applies to: 136-136, 290-290, 294-294
packages/mobile-sdk-alpha/src/processing/mrz.ts (1)
248-276: Validate actual calendar dates to prevent accepting impossible inputs (e.g., 2021-13-40).Currently, regex-only checks can pass invalid dates, which may cascade into downstream validation logic. Consider verifying month/day using Date to harden input handling.
Apply within this block:
export function formatDateToYYMMDD(inputDate: string): string { if (!inputDate || typeof inputDate !== 'string') { throw new MrzParseError('Date string is required'); } // Handle ISO date strings (YYYY-MM-DD format) const isoMatch = inputDate.match(/^(\d{4})-(\d{2})-(\d{2})/); if (isoMatch) { const [, year, month, day] = isoMatch; + // Validate calendar date + const d = new Date(`${year}-${month}-${day}T00:00:00Z`); + const valid = + !Number.isNaN(d.getTime()) && + d.getUTCFullYear() === Number(year) && + d.getUTCMonth() + 1 === Number(month) && + d.getUTCDate() === Number(day); + if (!valid) { + throw new MrzParseError(`Invalid date value: ${inputDate}`); + } return year.slice(2) + month + day; } // Handle other common formats const dateMatch = inputDate.match(/^(\d{2,4})[-/]?(\d{2})[-/]?(\d{2})/); if (dateMatch) { let [, year] = dateMatch; const [, , month, day] = dateMatch; // Handle 2-digit years (assume 20xx for 00-30, 19xx for 31-99) if (year.length === 2) { const yearNum = parseInt(year, 10); year = yearNum <= 30 ? `20${year}` : `19${year}`; } - - return year.slice(2) + month + day; + // Validate calendar date + const d = new Date(`${year}-${month}-${day}T00:00:00Z`); + const valid = + !Number.isNaN(d.getTime()) && + d.getUTCFullYear() === Number(year) && + d.getUTCMonth() + 1 === Number(month) && + d.getUTCDate() === Number(day); + if (!valid) { + throw new MrzParseError(`Invalid date value: ${inputDate}`); + } + return (year as string).slice(2) + month + day; } throw new MrzParseError(`Invalid date format: ${inputDate}. Expected ISO format (YYYY-MM-DD) or similar.`); }packages/mobile-sdk-alpha/tests/processing/nfc.test.ts (1)
25-28: Strengthen the assertion to verify error contract (code/category/retryable).Type-only assertions can miss regressions in error metadata. Asserting code/category makes the test guard the public error contract.
Apply this change:
it('throws on truncated data', () => { const bad = new Uint8Array([0x61, 0x05, 0x01]); - expect(() => parseNFCResponse(bad)).toThrowError(NfcParseError); + try { + parseNFCResponse(bad); + throw new Error('Expected NfcParseError'); + } catch (e) { + expect(e).toBeInstanceOf(NfcParseError); + expect((e as any).code).toBe('SELF_ERR_NFC_PARSE'); + expect((e as any).category).toBe('validation'); + expect((e as any).retryable).toBe(false); + } });packages/mobile-sdk-alpha/tests/processing/mrz.test.ts (1)
37-39: Assert error metadata to lock the public contractGiven the new typed-error taxonomy, also assert code/category to prevent regressions where a generic Error or wrong code slips in.
You can add a dedicated test case like:
it('exposes error code and category for MRZ parse failures', () => { try { extractMRZInfo('not-a-valid-mrz'); throw new Error('Expected failure'); } catch (err) { expect(err).toBeInstanceOf(MrzParseError); const e = err as { code?: string; category?: string }; expect(e.code).toBe('SELF_ERR_MRZ_PARSE'); expect(e.category).toBe('validation'); } });packages/mobile-sdk-alpha/src/processing/nfc.ts (2)
36-39: Classify TextDecoder absence as InitError, not NfcParseErrorDecoder availability is an environment/initialization issue, not a parsing failure. Using InitError here enables clearer upstream handling (e.g., retry vs. user input fix).
Apply:
-import { NfcParseError } from '../errors'; +import { NfcParseError, InitError } from '../errors'; @@ - throw new NfcParseError( + throw new InitError( 'TextDecoder not available in this environment. ' + 'This SDK requires TextDecoder support which is available in modern browsers, Node.js, and React Native.', );
97-101: Differentiate end-of-data errors to improve debuggabilityBoth guards throw 'Unexpected end of data'. Distinguishing the messages (e.g., 'after tag' vs. 'insufficient value bytes') will speed diagnosis without exposing sensitive data.
- if (i >= bytes.length) throw new NfcParseError('Unexpected end of data'); + if (i >= bytes.length) throw new NfcParseError('Unexpected end of data after tag byte'); @@ - if (i + length > bytes.length) throw new NfcParseError('Unexpected end of data'); + if (i + length > bytes.length) throw new NfcParseError('Unexpected end of data for value bytes');
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (24)
packages/mobile-sdk-alpha/README.md(1 hunks)packages/mobile-sdk-alpha/docs/ARCHITECTURE_CHECKLIST.md(1 hunks)packages/mobile-sdk-alpha/docs/errors.md(1 hunks)packages/mobile-sdk-alpha/src/errors/InitError.ts(1 hunks)packages/mobile-sdk-alpha/src/errors/LivenessError.ts(1 hunks)packages/mobile-sdk-alpha/src/errors/MrzParseError.ts(1 hunks)packages/mobile-sdk-alpha/src/errors/NfcParseError.ts(1 hunks)packages/mobile-sdk-alpha/src/errors/SdkError.ts(2 hunks)packages/mobile-sdk-alpha/src/errors/index.ts(1 hunks)packages/mobile-sdk-alpha/src/index.ts(1 hunks)packages/mobile-sdk-alpha/src/processing/mrz.ts(6 hunks)packages/mobile-sdk-alpha/src/processing/nfc.ts(4 hunks)packages/mobile-sdk-alpha/tests/processing/mrz.test.ts(3 hunks)packages/mobile-sdk-alpha/tests/processing/nfc.test.ts(2 hunks)sdk/core/index.ts(2 hunks)sdk/core/package.json(1 hunks)sdk/core/src/SelfBackendVerifier.ts(4 hunks)sdk/core/src/errors/ProofError.ts(1 hunks)sdk/core/src/errors/RegistryContractError.ts(1 hunks)sdk/core/src/errors/VerifierContractError.ts(1 hunks)sdk/core/src/errors/index.ts(1 hunks)sdk/core/src/utils/proof.ts(2 hunks)sdk/core/tests/proof.test.ts(1 hunks)sdk/core/tests/verifier-errors.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
sdk/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit Configuration File
sdk/**/*.{ts,tsx,js,jsx}: Review SDK packages for:
- Public API design and stability
- Asynchronous flows and error handling
- Security and input validation
- Compatibility across environments
Files:
sdk/core/src/errors/VerifierContractError.tssdk/core/index.tssdk/core/src/errors/RegistryContractError.tssdk/core/tests/verifier-errors.test.tssdk/core/src/errors/index.tssdk/core/src/errors/ProofError.tssdk/core/src/SelfBackendVerifier.tssdk/core/src/utils/proof.tssdk/core/tests/proof.test.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit Configuration File
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/errors/InitError.tspackages/mobile-sdk-alpha/src/errors/LivenessError.tspackages/mobile-sdk-alpha/src/processing/mrz.tspackages/mobile-sdk-alpha/src/errors/NfcParseError.tspackages/mobile-sdk-alpha/src/processing/nfc.tspackages/mobile-sdk-alpha/src/errors/index.tspackages/mobile-sdk-alpha/tests/processing/mrz.test.tspackages/mobile-sdk-alpha/tests/processing/nfc.test.tspackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/errors/SdkError.tspackages/mobile-sdk-alpha/src/errors/MrzParseError.ts
**/*.{test,spec}.{ts,js,tsx,jsx}
⚙️ CodeRabbit Configuration File
**/*.{test,spec}.{ts,js,tsx,jsx}: Review test files for:
- Test coverage completeness
- Test case quality and edge cases
- Mock usage appropriateness
- Test readability and maintainability
Files:
sdk/core/tests/verifier-errors.test.tspackages/mobile-sdk-alpha/tests/processing/mrz.test.tspackages/mobile-sdk-alpha/tests/processing/nfc.test.tssdk/core/tests/proof.test.ts
🧬 Code Graph Analysis (17)
sdk/core/src/errors/VerifierContractError.ts (2)
sdk/core/index.ts (1)
VerifierContractError(33-33)sdk/core/src/errors/index.ts (1)
VerifierContractError(32-32)
packages/mobile-sdk-alpha/src/errors/InitError.ts (1)
packages/mobile-sdk-alpha/src/errors/SdkError.ts (1)
SdkError(19-35)
packages/mobile-sdk-alpha/src/errors/LivenessError.ts (3)
packages/mobile-sdk-alpha/src/errors/index.ts (2)
LivenessError(3-3)SdkError(12-12)packages/mobile-sdk-alpha/src/index.ts (1)
LivenessError(47-47)packages/mobile-sdk-alpha/src/errors/SdkError.ts (1)
SdkError(19-35)
packages/mobile-sdk-alpha/src/processing/mrz.ts (3)
packages/mobile-sdk-alpha/src/errors/MrzParseError.ts (1)
MrzParseError(9-14)packages/mobile-sdk-alpha/src/errors/index.ts (1)
MrzParseError(4-4)packages/mobile-sdk-alpha/src/index.ts (1)
MrzParseError(48-48)
packages/mobile-sdk-alpha/src/errors/NfcParseError.ts (1)
packages/mobile-sdk-alpha/src/errors/SdkError.ts (1)
SdkError(19-35)
packages/mobile-sdk-alpha/src/processing/nfc.ts (3)
packages/mobile-sdk-alpha/src/errors/NfcParseError.ts (1)
NfcParseError(9-14)packages/mobile-sdk-alpha/src/errors/index.ts (1)
NfcParseError(5-5)packages/mobile-sdk-alpha/src/index.ts (1)
NfcParseError(49-49)
packages/mobile-sdk-alpha/src/errors/index.ts (1)
packages/mobile-sdk-alpha/src/index.ts (1)
SCANNER_ERROR_CODES(50-50)
sdk/core/src/errors/RegistryContractError.ts (2)
sdk/core/index.ts (1)
RegistryContractError(32-32)sdk/core/src/errors/index.ts (1)
RegistryContractError(31-31)
sdk/core/tests/verifier-errors.test.ts (4)
sdk/core/src/errors/RegistryContractError.ts (1)
RegistryContractError(6-11)sdk/core/index.ts (2)
RegistryContractError(32-32)VerifierContractError(33-33)sdk/core/src/errors/index.ts (2)
RegistryContractError(31-31)VerifierContractError(32-32)sdk/core/src/errors/VerifierContractError.ts (1)
VerifierContractError(6-11)
packages/mobile-sdk-alpha/tests/processing/mrz.test.ts (2)
packages/mobile-sdk-alpha/src/processing/mrz.ts (2)
extractMRZInfo(206-242)formatDateToYYMMDD(248-276)packages/mobile-sdk-alpha/src/errors/MrzParseError.ts (1)
MrzParseError(9-14)
packages/mobile-sdk-alpha/tests/processing/nfc.test.ts (2)
packages/mobile-sdk-alpha/src/processing/nfc.ts (1)
parseNFCResponse(92-120)packages/mobile-sdk-alpha/src/errors/NfcParseError.ts (1)
NfcParseError(9-14)
sdk/core/src/errors/ProofError.ts (2)
sdk/core/index.ts (1)
ProofError(34-34)sdk/core/src/errors/index.ts (1)
ProofError(33-33)
sdk/core/src/SelfBackendVerifier.ts (4)
sdk/core/src/errors/RegistryContractError.ts (1)
RegistryContractError(6-11)sdk/core/index.ts (2)
RegistryContractError(32-32)VerifierContractError(33-33)sdk/core/src/errors/index.ts (2)
RegistryContractError(31-31)VerifierContractError(32-32)sdk/core/src/errors/VerifierContractError.ts (1)
VerifierContractError(6-11)
sdk/core/src/utils/proof.ts (2)
sdk/core/src/errors/ProofError.ts (1)
ProofError(6-11)sdk/core/src/errors/index.ts (1)
ProofError(33-33)
packages/mobile-sdk-alpha/src/errors/SdkError.ts (1)
packages/mobile-sdk-alpha/src/errors/index.ts (3)
SdkErrorCategory(12-12)notImplemented(12-12)SdkError(12-12)
packages/mobile-sdk-alpha/src/errors/MrzParseError.ts (3)
packages/mobile-sdk-alpha/src/errors/index.ts (2)
MrzParseError(4-4)SdkError(12-12)packages/mobile-sdk-alpha/src/index.ts (1)
MrzParseError(48-48)packages/mobile-sdk-alpha/src/errors/SdkError.ts (1)
SdkError(19-35)
sdk/core/tests/proof.test.ts (2)
sdk/core/src/utils/proof.ts (1)
getRevealedDataPublicSignalsLength(14-23)sdk/core/src/errors/ProofError.ts (1)
ProofError(6-11)
🔇 Additional comments (14)
sdk/core/index.ts (1)
10-16: LGTM: re-exporting typed errors via public APIPublic API exposure of error types is consistent and useful for consumer-side error handling. No issues.
Also applies to: 30-35
packages/mobile-sdk-alpha/README.md (1)
50-59: Typed error docs align with the new API surface.The section correctly reflects the new error classes, categories, and shared SdkError fields. Looks consistent with the implementation and exports.
packages/mobile-sdk-alpha/docs/errors.md (1)
1-12: Error catalog looks accurate and consistent with implementation.Codes, categories, and class names match the code. Good move toward clearer, typed failure modes.
packages/mobile-sdk-alpha/src/processing/mrz.ts (1)
20-24: Replacing generic throw with MrzParseError is correct and safer.Error type and message are specific without leaking MRZ content—good balance for security and debuggability.
packages/mobile-sdk-alpha/docs/ARCHITECTURE_CHECKLIST.md (1)
25-29: Checklist updates accurately reflect the typed error migration.Marking exception classes complete with the new error types aligns with the code changes in this PR.
packages/mobile-sdk-alpha/tests/processing/mrz.test.ts (1)
3-3: Importing MrzParseError from public API is correctGood to see tests relying on the public surface (../../src) for typed errors.
packages/mobile-sdk-alpha/src/processing/nfc.ts (1)
67-79: Robust length parsing with clear error signalingBounds checks for short/long-form lengths and indefinite-length rejection look correct and protect against out-of-bounds reads.
packages/mobile-sdk-alpha/src/errors/InitError.ts (1)
9-14: Typed InitError aligns with the new error taxonomyConstructor signature and base SdkError wiring look correct.
packages/mobile-sdk-alpha/src/errors/LivenessError.ts (1)
9-14: Typed LivenessError is wired correctlyExtends SdkError with category 'liveness' and stable code; looks good.
packages/mobile-sdk-alpha/src/errors/NfcParseError.ts (1)
9-14: NfcParseError wiring is correct and consistentConstructor, code, category, and name are aligned with the NFC parsing domain.
packages/mobile-sdk-alpha/src/errors/MrzParseError.ts (1)
9-14: Typed MRZ error is consistent and well-scopedGood use of a domain-specific error with a stable code, correct category ('validation'), and non-retryable flag. Name override is correct for instanceof checks.
packages/mobile-sdk-alpha/src/index.ts (1)
45-53: Public API surfaces the new typed errors — looks goodExporting InitError, LivenessError, MrzParseError, and NfcParseError through the top-level barrel aligns API ergonomics with the new error taxonomy and avoids deep imports.
packages/mobile-sdk-alpha/src/errors/index.ts (1)
6-10: Good move extracting SCANNER_ERROR_CODES to the errors barrelCentralizing these scanner codes here is consistent with the new error organization and reduces duplication.
packages/mobile-sdk-alpha/src/errors/SdkError.ts (1)
5-14: Category additions align with new error classesIncluding 'init' and 'liveness' in SdkErrorCategory matches the new domain-specific errors and improves downstream handling.
|
this is ready for review. going to merge the updated docs after - #922 |
Summary
Testing
yarn lint(fails: Unexpected any / console warnings)yarn buildyarn workspace @selfxyz/contracts build(fails: Hardhat config accounts undefined)yarn typesyarn test(fails: failing packages @selfxyz/mobile-app, @selfxyz/circuits, @selfxyz/contracts, scripts/tests, @selfxyz/core)yarn workspace @selfxyz/mobile-sdk-alpha niceyarn workspace @selfxyz/mobile-sdk-alpha typesyarn workspace @selfxyz/mobile-sdk-alpha buildyarn workspace @selfxyz/mobile-sdk-alpha validate:exportsyarn workspace @selfxyz/mobile-sdk-alpha validate:pkgyarn workspace @selfxyz/mobile-sdk-alpha testyarn workspace @selfxyz/core testhttps://chatgpt.com/codex/tasks/task_b_68a2a944aa50832dafea04086e215fca
Summary by CodeRabbit