refactor: decouple error classes from i18n + conventional locale structure#2673
Open
goastler wants to merge 20 commits into
Open
refactor: decouple error classes from i18n + conventional locale structure#2673goastler wants to merge 20 commits into
goastler wants to merge 20 commits into
Conversation
Errors now carry a raw translation key and context; translation happens at the API/UI boundary (unwrapError, handleErrors) and logging is the caller's responsibility. Removes i18n and logger options from BaseErrorOptions and all construction sites.
- Remove TranslationKey and TFunction imports from @prosopo/common; error constructors now accept plain string instead of the branded type - Drop @prosopo/locale and i18next from common's dependencies - Remove i18n parameter from unwrapError; message field is now the raw translation key so translation happens at the UI/response layer only - Fix i18SharedOptions: namespace → defaultNS, add explicit ns list - i18nBackend.initializeI18n now throws when called in browser context instead of silently returning an uninitialised instance - Remove request.i18n from handleErrors; update errorHandler tests to expect key strings rather than translated messages - ProcaptchaWidget translates error.key via useTranslation at render time
getLeafFieldPath was returning [] for string leaf nodes, causing the runtime schema to have zero entries and TranslationKey to resolve to string rather than the specific union of all valid keys. Fix the runtime function by returning [""] as a sentinel for leaf nodes (parent joins with the key, omitting the separator for leaves). Add a Leaves<T> recursive conditional type that TypeScript evaluates at compile time against the literal JSON shape, giving the exact union independently of the runtime function. Co-Authored-By: George Oastler <goastler4@gmail.com>
Adds 18 keys that were used in error constructors but absent from the translation file, causing i18next to return raw key strings to users: - BILLING.STRIPE_PORTAL/SESSION_NOT_FOUND/STRIPE_PAYMENT_FAILED (new section) - API.FEATURE_NOT_ENABLED/INVALID_AUTHORIZATION_HEADER/INVALID_DOMAIN/NO_PERMISSIONS - PORTAL.CANNOT_MODIFY_USER/API_KEYS_INVALID_NUMBER_OF_SITES/API_KEYS_NOT_FOUND - GENERAL.BILLING_ERROR/INVALID_JWT/MISSING_AUTH_HEADER/OBJECT_COUNT_ERROR/SECRET_MISSING - DEVELOPER.MISSING_SECRET_KEY - CAPTCHA.FAILED/PASSED Other locales fall back to English via i18next fallbackLng. Co-Authored-By: George Oastler <goastler4@gmail.com>
Add @prosopo/locale devDependency to common package and type all error constructor first arguments as Error | TranslationKey, replacing the previous untyped string. This ensures translation keys are validated at compile time against the known translation JSON.
…anslations The plugin only retains translation keys that appear as string literals in the bundled source. Keys built at runtime (template literals, API responses, variable lookups) are silently stripped. Add a comment making this constraint explicit so callers know to list such keys statically.
Replace the overloaded loadI18next(boolean) with explicit loadI18nextFrontend() and loadI18nextBackend() exports. This removes the boolean flag footgun (callers could accidentally pass the wrong value) and also fixes a race condition where concurrent calls before the first promise resolved would invoke initializeI18n twice. The original loadI18next(boolean) is kept as a thin wrapper for compatibility.
…ndition fixes Add message field to ProsopoBaseError to provide both translation keys and English fallback messages. Update Vite plugin to preserve backend error keys. Fix i18n initialization race conditions. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Create ValidErrorKey type union derived from const assertions to enforce type safety on all error keys. Update error factories and Vite config to use constants instead of string literals. Single source of truth for all valid backend error keys prevents typos and drift. - Add errorKeys.ts with API_ERROR_KEYS, GENERAL_ERROR_KEYS, PORTAL_ERROR_KEYS, BILLING_ERROR_KEYS - Generate ValidErrorKey type for compile-time validation - Update ProsopoApiError to accept only ValidErrorKey - Update 17 error factories to use constants - Update Vite config to import BACKEND_ERROR_KEYS_ARRAY - Export new error key constants from common package
Validate that all ValidErrorKey values exist in translation.json at compile time. Imports the existing TranslationKey type derived from the English translation JSON and uses it to ensure no error key is missing from translations. This catches cases where you define an error key in errorKeys.ts but forget to add it to translation.json - TypeScript will error at build time instead of failing at runtime.
Keep type-safe error key constants while integrating with main branch's refactored structure. Merge translations from both branches. - Resolve error.ts conflicts: keep hybrid pattern with message field - Update package.json: keep @prosopo/locale, remove unnecessary i18n/logger deps - Update index.ts: export errorKeys alongside refactored utils structure - Resolve cli.ts: keep both loadI18nextBackend and logger - Resolve getFrictionlessCaptchaChallenge: use refactored handler export - Merge translation.json: combine all keys from both branches
- ProsopoApiError accepts TranslationKey again (validation reasons are arbitrary translation keys); ValidErrorKey stays the curated bundle list - Remove obsolete i18n/logger constructor options from error call sites - Drop redundant forwarding constructors on Prosopo*Error subclasses - disapproveDappUserCommitment reason typed as ResultReason (matches storage) - Add missing backend translation keys (DATABASE.*, API.*, CAPTCHA.*) and propagate the full en key set to all 32 locales so key sets stay in sync - Rewrite common error tests for the decoupled API; fix api-express-router error handler/auth middleware tests for the new error shape
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the backend/common error model to be independent of i18n/logging at construction time, while also normalizing i18n loading APIs and ensuring backend-used translation keys exist across all locale files (and are preserved in frontend bundles).
Changes:
- Refactored
Prosopo*Errorclasses andunwrapErrorso errors carry atranslationKey+ fallbackmessage, and logging is performed by request-level loggers / callers. - Introduced a curated backend error-key registry (
BACKEND_ERROR_KEYS_ARRAY) and updated the Vite translation-pruning plugin to always retain backend-returned keys in frontend bundles. - Reworked i18n loader entrypoints (
loadI18nextFrontend/loadI18nextBackend) and propagated missing backend keys into all locale JSONs to satisfy key-consistency.
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types-database/src/types/provider.ts | Tightens disapproveDappUserCommitment reason typing to persisted ResultReason. |
| packages/provider/src/tests/unit/api/validateAddress.unit.test.ts | Updates test to use an existing/expected API translation key. |
| packages/provider/src/tasks/imgCaptcha/imgCaptchaTasks.ts | Uses ResultReason.CAPTCHA_INVALID_SOLUTION instead of a raw translation key. |
| packages/provider/src/tasks/client/clientTasks.ts | Removes logger injection into errors; normalizes detector-key error key namespace. |
| packages/provider/src/api/verify.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/validateAddress.ts | Stops passing logger into errors; relies on upstream logging/error handling. |
| packages/provider/src/api/domainMiddleware.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/captcha/submitPuzzleCaptchaSolution.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/captcha/submitPoWCaptchaSolution.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/captcha/submitImageCaptchaSolution.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/captcha/getPuzzleCaptchaChallenge.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/captcha/getPoWCaptchaChallenge.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/captcha/getImageCaptchaChallenge.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/captcha/getFrictionlessCaptchaChallenge/handler.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/captcha/getFrictionlessCaptchaChallenge/decisionMachine.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/provider/src/api/captcha/checkSpamEmail.ts | Removes i18n/logger from ProsopoApiError construction (decoupling). |
| packages/procaptcha-react/src/components/ProcaptchaWidget.tsx | Switches to loadI18nextFrontend; translates widget error via returned key. |
| packages/procaptcha-pow/src/components/ProcaptchaWidget.tsx | Switches to loadI18nextFrontend for frontend i18n initialization. |
| packages/procaptcha-frictionless/src/ProcaptchaFrictionless.tsx | Switches to loadI18nextFrontend for frontend i18n initialization. |
| packages/procaptcha-bundle/vite.config.ts | Passes BACKEND_ERROR_KEYS_ARRAY to ensure backend keys aren’t pruned from bundles. |
| packages/procaptcha-bundle/src/util/widgetFactory.ts | Uses loadI18nextFrontend instead of backend/flag-based loader. |
| packages/locale/src/translationKey.ts | Fixes runtime leaf-path generation; derives TranslationKey union from JSON shape. |
| packages/locale/src/locales/en/translation.json | Adds missing backend keys and new sections to the canonical locale. |
| packages/locale/src/locales/zh-CN/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/vi/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/uk/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/tr/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/th/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/sv/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/sr/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/ru/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/ro/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/pt/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/pt-BR/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/pl/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/no/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/nl/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/ms/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/ml/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/ko/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/jv/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/ja/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/it/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/id/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/hu/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/hi/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/fr/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/fi/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/es/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/el/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/de/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/cs/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/az/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/locales/ar/translation.json | Adds missing keys (English placeholders where untranslated) to match en. |
| packages/locale/src/loadI18next.ts | Splits frontend/backend i18n loaders; adds caching and race prevention. |
| packages/locale/src/index.ts | Re-exports new loader entrypoints (loadI18nextFrontend/Backend). |
| packages/locale/src/i18SharedOptions.ts | Uses conventional i18next defaultNS + ns options. |
| packages/locale/src/i18nMiddleware.ts | Uses loadI18nextBackend for server middleware initialization. |
| packages/locale/src/i18nBackend.ts | Enforces server-only usage and removes redundant NS configuration. |
| packages/env/src/env.ts | Removes logger injection into ProsopoEnvError construction. |
| packages/database/src/databases/provider.ts | Removes logger injection into ProsopoDBError construction; uses explicit logger calls. |
| packages/database/src/databases/client.ts | Removes logger injection into ProsopoDBError construction. |
| packages/database/src/databases/captcha.ts | Removes logger injection into ProsopoDBError construction. |
| packages/database/src/base/mongo.ts | Removes logger injection into ProsopoDBError construction. |
| packages/common/src/tests/error.unit.test.ts | Rewrites tests for the decoupled error API and updated unwrap/log behavior. |
| packages/common/src/index.ts | Exports new error-key registry module. |
| packages/common/src/errorKeys.ts | Adds backend error-key registry and compile-time validation hook. |
| packages/common/src/error.ts | Refactors error classes + unwrapError to remove i18n/logger coupling. |
| packages/common/package.json | Drops now-unneeded runtime deps (@prosopo/logger, i18next) from @prosopo/common. |
| packages/cli/src/cli.ts | Uses loadI18nextBackend for CLI/server i18n initialization. |
| packages/api-express-router/src/tests/unit/middlewares/authMiddleware.unit.test.ts | Updates auth middleware tests for new error shape (no request i18n in context). |
| packages/api-express-router/src/tests/unit/errorHandler.unit.test.ts | Updates error handler tests for new unwrap/log behavior. |
| packages/api-express-router/src/middlewares/authMiddleware.ts | Removes i18n from error context; relies on explicit logging + unwrap. |
| packages/api-express-router/src/errorHandler.ts | Logs with request-level logger and calls unwrapError without i18n. |
| dev/config/src/vite/vite-plugin-remove-unused-translations.ts | Preserves backend error keys during translation key pruning. |
| demos/provider-mock/src/db.ts | Removes logger injection into ProsopoDBError construction. |
| .changeset/decouple-error-i18n.md | Documents the behavioral/API changes across packages and bumps versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+81
to
+87
| // Type-level validation: ensure all error keys exist in translation.json | ||
| // Each ValidErrorKey must be assignable to TranslationKey (derived from translation JSON) | ||
| // If this errors, a key is defined here but missing from the translation file. | ||
| export const _validateErrorKeysExistInTranslations: Record<ValidErrorKey, unknown> = {} as Record< | ||
| ValidErrorKey, | ||
| TranslationKey | ||
| >; |
Comment on lines
48
to
53
| resolve(frontendInstance); | ||
| } | ||
| } catch (e) { | ||
| frontendInitialized = false; | ||
| reject(e); | ||
| } |
Comment on lines
86
to
92
| } else if (backendInstance) { | ||
| resolve(backendInstance); | ||
| } | ||
| } catch (e) { | ||
| backendInitialized = false; | ||
| reject(e); | ||
| } |
…or keys in integration tests - Sync package.json deps with tsconfig references (lint:refs): remove unused @prosopo/locale from api-express-router; add @prosopo/logger devDep to common; add @prosopo/common to procaptcha-bundle (+ lockfile) - Remove obsolete logger option from client-example-server demo error - Integration tests now assert the locale-stable error key (decoupled errors return the translation key, not the translated message)
Comment on lines
+27
to
+28
| request.logger.error(() => ({ err })); | ||
| const { code, statusMessage, jsonError } = unwrapError(err); |
Comment on lines
+81
to
+87
| // Type-level validation: ensure all error keys exist in translation.json | ||
| // Each ValidErrorKey must be assignable to TranslationKey (derived from translation JSON) | ||
| // If this errors, a key is defined here but missing from the translation file. | ||
| export const _validateErrorKeysExistInTranslations: Record< | ||
| ValidErrorKey, | ||
| unknown | ||
| > = {} as Record<ValidErrorKey, TranslationKey>; |
Comment on lines
32
to
40
| if (!valid) { | ||
| throw new ProsopoApiError(translationKey, { | ||
| context: { code: 400, siteKey: address }, | ||
| logger, | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| throw new ProsopoApiError(translationKey, { | ||
| context: { code: 400, siteKey: address }, | ||
| logger, | ||
| }); |
Production code never constructs an ApiError from a raw Error; doing so leaked the internal error message to the HTTP response with no client key. Restricting the first arg to TranslationKey enforces that every API error carries an intentional, localizable key. Error chaining still flows through context.error, which unwrapError walks. Base ProsopoBaseError keeps Error | TranslationKey for internal/CLI errors that wrap raw Errors.
Comment on lines
+25
to
+27
| export async function loadI18nextFrontend(): Promise<i18n> { | ||
| if (frontendInstance) return frontendInstance; | ||
| if (!frontendPromise) { |
Comment on lines
+64
to
+66
| export async function loadI18nextBackend(): Promise<i18n> { | ||
| if (backendInstance) return backendInstance; | ||
| if (!backendPromise) { |
Comment on lines
+84
to
+87
| export const _validateErrorKeysExistInTranslations: Record< | ||
| ValidErrorKey, | ||
| unknown | ||
| > = {} as Record<ValidErrorKey, TranslationKey>; |
Comment on lines
88
to
92
| constructor( | ||
| error: Error | TranslationKey, | ||
| translationKey: TranslationKey, | ||
| options?: BaseErrorOptions<ApiContextParams>, | ||
| ) { | ||
| const errorName = options?.name || "ProsopoApiError"; | ||
| const code = options?.context?.code || 500; |
Comment on lines
32
to
36
| if (!valid) { | ||
| throw new ProsopoApiError(translationKey, { | ||
| context: { code: 400, siteKey: address }, | ||
| logger, | ||
| }); | ||
| } |
…ional cause Every Prosopo*Error now takes a required TranslationKey as its first arg and an optional causing Error via options.cause (its message becomes the fallback message). Removes the mutually-exclusive Error|TranslationKey first arg. - Base constructor + BaseErrorOptions reworked (drop options.translationKey, add options.cause) - Migrated datasets-fs (16 sites) to key-first + message, keeping their existing FS.*/DATASET.* keys - env getters and authMiddleware migrated; keyless internal errors use the new GENERAL.UNKNOWN placeholder with detail in options.message - Added GENERAL.UNKNOWN to all 32 locales - Updated common error tests for the new model
- errorKeys: make _validateErrorKeysExistInTranslations a real compile-time check (assign BACKEND_ERROR_KEYS_ARRAY to readonly TranslationKey[]) instead of a no-op cast - loadI18next: only short-circuit once the instance is initialized (avoid handing back a half-ready i18n to concurrent callers); clear the cached promise on failure so initialization can be retried - errorHandler: guard request.logger?.error so a missing request-logger middleware doesn't throw and mask the original error - validateAddr: mark the now-unused logger param as _logger
Member
Author
|
Thanks for the review — addressed all comments in 46b2538:
|
Comment on lines
77
to
+80
| if (!overwrite && fs.existsSync(outDir)) { | ||
| throw new ProsopoEnvError( | ||
| new Error(`Output directory already exists: ${outDir}`), | ||
| { | ||
| translationKey: "FS.FILE_NOT_FOUND", | ||
| }, | ||
| ); | ||
| throw new ProsopoEnvError("FS.FILE_NOT_FOUND", { | ||
| message: `Output directory already exists: ${outDir}`, | ||
| }); |
Comment on lines
+38
to
+43
| export const GENERAL_ERROR_KEYS = { | ||
| ACCOUNT_NOT_FOUND: "GENERAL.ACCOUNT_NOT_FOUND", | ||
| SITE_KEY_NOT_FOUND: "GENERAL.SITE_KEY_NOT_FOUND", | ||
| MISSING_SECRET_KEY: "GENERAL.MISSING_SECRET_KEY", | ||
| INVALID_SIGNATURE: "GENERAL.INVALID_SIGNATURE", | ||
| } as const; |
- resize: use FS.FILE_ALREADY_EXISTS when output dir exists (was FILE_NOT_FOUND) - errorKeys: preserve GENERAL.MISSING_AUTH_HEADER/INVALID_JWT/UNKNOWN in the frontend bundle (thrown by backend, returned to frontend as the error key)
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.
Summary
Decouples the
Prosopo*Errorclasses from i18n and logging, moves translations into a conventional i18n structure, and normalizes the error-class API so every error has a queryable translation key.Error model
ProsopoBaseErrorand subclasses no longer translate or log at construction time. They carry atranslationKeyplus a fallbackmessage; translation happens at the presentation layer (UI render or HTTP response viaunwrapError).TranslationKeyfirst arg and an optional causingErrorviaoptions.cause(whose message becomes the fallback). Removed the oldError | TranslationKeyfirst arg,options.translationKey, and thei18n/logger/logLeveloptions.ProsopoApiErroracceptsTranslationKeyonly (notError) — passing a rawErrorleaked internal text into HTTP responses and produced no client key.ValidErrorKey/BACKEND_ERROR_KEYS_ARRAYremain the curated set preserved in the frontend bundle by the Vite plugin.GENERAL.UNKNOWNplaceholder with the detail inoptions.message(e.g. env "not setup" guards). datasets-fs sites kept their existingFS.*/DATASET.*keys.Type-safe translation keys
disapproveDappUserCommitment(reason?)is typedResultReason(matching the persistedresult.reason).enkey set to all 32 locales so the locale-consistency test passes.Tests
@prosopo/commonerror tests for the decoupled/uniform API.@prosopo/api-express-routerand provider integration tests for the new error shape (responses carry the locale-stable key, not translated text).Test plan
build:tscgreen across all buildable backend/non-frontend packages.@prosopo/common,@prosopo/locale,@prosopo/database,@prosopo/api-express-router,@prosopo/logger,@prosopo/env,@prosopo/datasets-fsunit tests pass; full@prosopo/providerunit suite (785) passes.