[RESEARCH ONLY] Neverthrow

neverthrow.md

@@ -0,0 +1,106 @@
+# Try/Catch → neverthrow Feasibility Analysis
+
+## Context
+
+You asked how many of the repo's `try/catch` blocks could realistically be simplified by adopting
+[`neverthrow`](https://github.com/supermacro/neverthrow). This is a scoping report, not an execution plan — it should
+let you decide whether/where adopting neverthrow is worth the dependency and refactor cost before committing to any code
+changes.
+
+## Headline numbers
+
+- **63 try/catch blocks** across **838 TS/JS source files** (~7.5% file prevalence)
+- **`neverthrow` is not a current dependency** anywhere in the monorepo
+- **No existing custom Result/Either utility** — each package defines its own ad-hoc shapes (`ApiResponse<T>`,
+  `CommandResult`, …)
+- TS posture is friendly to it: TS 5.9, `strict: true`, library-style packaging with declaration files
+
+## Categorization
+
+| Category | Description                                                                         |   Count | Good fit for neverthrow?                                     |
+| -------- | ----------------------------------------------------------------------------------- | ------: | ------------------------------------------------------------ |
+| A        | Single fallible call wrapped to transform/handle error                              | **~28** | ✅ Best candidates — `ResultAsync.fromPromise` one-liners    |
+| B        | Multiple chained fallible operations in one `try`                                   |  **~8** | ✅ Idiomatic `andThen` chains                                |
+| C        | Boundary / framework integration (CLI entry, React Query `queryFn`, OAuth boundary) | **~15** | ⚠️ Usually must stay — these are where Results get unwrapped |
+| D        | Resource cleanup (`finally`) or intentional silent-swallow                          |  **~8** | ⚠️ Convertible but loses ergonomics; case-by-case            |
+| E        | Tests / ambiguous                                                                   |  **~4** | n/a                                                          |
+
+**Convertible without changing public API**: ~30–36 blocks (A + most of B). **Convertible only as part of a major
+version bump**: most of category C if you also redesign CQRS client return types.
+
+## Per-package hotspots
+
+| Package                                                | try/catch | Notes                                                                       |
+| ------------------------------------------------------ | --------: | --------------------------------------------------------------------------- |
+| [packages/intl](packages/intl)                         |        20 | POEditor client + CLI commands — densest, most uniform Category A wrappers  |
+| [packages/kratos](packages/kratos)                     |        18 | Mostly Category C (React Query `queryFn`s) + 4 silent-swallow Passkey calls |
+| [packages/mail-translation](packages/mail-translation) |         7 | Mostly Category A — error-message-rewriting wrappers                        |
+| [packages/login-manager](packages/login-manager)       |         4 | OAuth boundary — Category C                                                 |
+| Other                                                  |        14 | Scattered single-use                                                        |
+
+## Highest-leverage targets (start here if you proceed)
+
+These are the cleanest Category A blocks — converting them gives the most "look at the diff" payoff for the least risk,
+because they're internal to their packages and don't change cross-package surfaces:
+
+1. [packages/intl/src/poeditor/POEditorClient.ts:43-59](packages/intl/src/poeditor/POEditorClient.ts#L43) — 5 methods,
+   each wraps a single API call to rethrow with context. Textbook `ResultAsync.fromPromise(call, mapErr)`.
+2. [packages/mail-translation/src/compileMjml.ts:18](packages/mail-translation/src/compileMjml.ts#L18) and
+   [packages/mail-translation/src/loadConfig.ts:36](packages/mail-translation/src/loadConfig.ts#L36) — single-call
+   wrappers that enhance error messages.
+3. [packages/intl/src/formatjs.ts:17](packages/intl/src/formatjs.ts#L17) — two CLI command wrappers.
+4. [packages/image-uploader/src/lib/\_utils/tryUploadWithUploadParams.ts:37-55](packages/image-uploader/src/lib/_utils/tryUploadWithUploadParams.ts#L37)
+   — currently swallows original error; Result preserves it without uglifying control flow.
+5. [packages/intl/src/commands/download.ts:17-49](packages/intl/src/commands/download.ts#L17) and
+   [packages/intl/src/commands/local.ts:30-89](packages/intl/src/commands/local.ts#L30) — Category B chains that would
+   read well as `.andThen(...).andThen(...)`.
+
+## What should _not_ move
+
+- **CQRS client return types** ([fetch-client](packages/cqrs-clients/fetch-client),
+  [axios-cqrs-client](packages/cqrs-clients/axios-cqrs-client), [rx-cqrs-client](packages/cqrs-clients/rx-cqrs-client),
+  [react-query-cqrs-client](packages/cqrs-clients/react-query-cqrs-client)) — these are public APIs of 6 published
+  packages; changing them is a major version bump and overlaps the existing `ApiResponse<T>` / `CommandResult` design
+  from [cqrs-client-base](packages/cqrs-clients/cqrs-client-base).
+- **rx-cqrs-client** — RxJS has its own error channel; mixing `Result` inside Observables adds cognitive overhead with
+  little gain.
+- **React Query `queryFn` handlers in kratos**
+  ([packages/kratos/src/lib/flows/login/hooks/useGetLoginFlow.ts:22-30](packages/kratos/src/lib/flows/login/hooks/useGetLoginFlow.ts#L22)
+  and ~9 siblings) — React Query _expects_ throws to drive its error state, so converting these gains nothing.
+- **Silent-swallow passkey calls**
+  ([packages/kratos/src/lib/utils/passkeys/index.ts:22-37](packages/kratos/src/lib/utils/passkeys/index.ts#L22)) — these
+  are _intentionally_ returning `undefined` on error. Convertible to `Result`, but only if you actually want callers to
+  handle the absence explicitly.
+- **CLI top-level entry points** in [packages/intl/src/commands/](packages/intl/src/commands/) — these are where Results
+  get unwrapped to exit codes; they should remain try/catch (or `.match()`).
+
+## Honest assessment
+
+**~36 of 63 blocks (≈57%) are technically convertible** without touching public APIs. Of those, **~10–12 would
+meaningfully improve readability or correctness** (the Category A wrappers in `intl/poeditor` and `mail-translation`,
+plus the swallow-bug in `image-uploader`). The rest are short single-use blocks where neverthrow adds a dependency for
+marginal stylistic win.
+
+**Recommendation**: this isn't a "rip out all try/catch" situation. If you want to adopt neverthrow:
+
+- Pilot it in **`packages/intl`** only (densest cluster, no external API breakage), specifically
+  `poeditor/POEditorClient.ts` + the two CLI command files. ~15 blocks converted, 1 package gets the dep, you can
+  reverse course cheaply if the team dislikes it.
+- Re-evaluate after that pilot before touching `mail-translation`, `image-uploader`, or any CQRS client.
+
+## What I would not recommend
+
+- Adding neverthrow as a peer dep of `cqrs-client-base` or any of the CQRS clients in this pass — that's a multi-package
+  coordinated breaking change masquerading as a refactor.
+- Converting the kratos React Query `queryFn`s — net negative.
+
+## Verification of this report
+
+Findings are derived from a ripgrep + read sweep over the repo. To spot-check:
+
+```
+rg -n --type ts --type tsx -g '!**/node_modules' -g '!**/dist' -g '!**/coverage' '\bcatch\s*\(' packages | wc -l
+rg -n --type ts --type tsx -g '!**/node_modules' '"neverthrow"' .
+```
+
+The first should land near 63; the second should return nothing (confirming it isn't currently a dep).

packages/image-uploader/package.json

@@ -23,6 +23,7 @@
   },
   "dependencies": {
     "@leancodepl/utils": "10.3.1",
+    "neverthrow": "^8.2.0",
     "react-dropzone": "14.3.8",
     "react-easy-crop": "5.5.0",
     "uuid": "11.1.0"

packages/image-uploader/src/index.ts

@@ -1,5 +1,6 @@
 export { ErrorCode } from "./lib/_utils/errors"
 export * from "./lib/_utils/tryUploadWithUploadParams"
+export { formatUploadError, type UploadError } from "./lib/_utils/UploadError"
 export * from "./lib/UploadImages"
 export * from "./lib/types"
 export { useUploadImages, type UseUploadImagesProps } from "./lib/_hooks/useUploadImages"

packages/image-uploader/src/lib/_utils/UploadError.ts

@@ -0,0 +1,28 @@
+export type UploadError =
+  | { kind: "getUploadParamsFailed"; cause: unknown }
+  | { kind: "imageNotDefined" }
+  | { kind: "noUploadParams" }
+  | { kind: "uploadRequestFailed"; cause: unknown }
+  | { kind: "uploadResponseNotOk"; status: number; statusText: string }
+
+export function formatUploadError(error: UploadError): string {
+  switch (error.kind) {
+    case "imageNotDefined":
+      return "Image is not defined"
+    case "getUploadParamsFailed":
+      return `Failed to obtain upload params: ${stringifyCause(error.cause)}`
+    case "noUploadParams":
+      return "Upload params are not defined"
+    case "uploadRequestFailed":
+      return `Upload request failed: ${stringifyCause(error.cause)}`
+    case "uploadResponseNotOk":
+      return `Upload server responded with ${error.status} ${error.statusText}`
+  }
+}
+
+function stringifyCause(cause: unknown): string {
+  if (cause instanceof Error) {
+    return cause.message
+  }
+  return String(cause)
+}

packages/image-uploader/src/lib/_utils/tryUploadWithUploadParams.ts

@@ -1,4 +1,6 @@
+import { errAsync, okAsync, ResultAsync } from "neverthrow"
 import { FileWithId, UploadedFileWithId, UploadParams } from "../types"
+import { UploadError } from "./UploadError"
 
 /**
  * Uploads an image file using provided upload parameters.
@@ -8,49 +10,60 @@ import { FileWithId, UploadedFileWithId, UploadParams } from "../types"
  *
  * @param image - Image file with ID or already uploaded image with URL
  * @param getUploadParams - Function that returns upload parameters (URI, method, headers) for the image
- * @returns Promise resolving to uploaded image with URL
- * @throws {Error} When upload params are not defined, image is not defined, or upload fails
+ * @returns ResultAsync resolving to the uploaded image with URL, or a typed `UploadError` on failure
  *
  * @example
  * ```typescript
  * import { tryUploadWithUploadParams } from "@leancodepl/image-uploader";
  *
- * const uploadedImage = await tryUploadWithUploadParams(image, async (img) => ({
+ * const result = await tryUploadWithUploadParams(image, async (img) => ({
  *   uri: "https://api.example.com/upload",
  *   method: "POST",
  *   requiredHeaders: { "Content-Type": "image/jpeg" }
  * }));
+ *
+ * if (result.isErr()) {
+ *   // result.error is a typed UploadError; switch on result.error.kind
+ * } else {
+ *   const uploadedImage = result.value;
+ * }
  * ```
  */
-export async function tryUploadWithUploadParams(
+export function tryUploadWithUploadParams(
   image: FileWithId | UploadedFileWithId,
   getUploadParams: (image: FileWithId) => Promise<UploadParams | null | undefined>,
-): Promise<UploadedFileWithId> {
+): ResultAsync<UploadedFileWithId, UploadError> {
   if ("url" in image) {
-    return image
+    return okAsync(image)
   }
 
   if (!image.originalFile) {
-    throw new Error("Image is not defined")
+    return errAsync({ kind: "imageNotDefined" })
   }
 
-  try {
-    const uploadParams = await getUploadParams(image)
+  const fileImage = image
 
+  return ResultAsync.fromPromise(
+    getUploadParams(fileImage),
+    (cause): UploadError => ({ kind: "getUploadParamsFailed", cause }),
+  ).andThen(uploadParams => {
     if (!uploadParams) {
-      throw new Error("Upload params are not defined")
+      return errAsync<UploadedFileWithId, UploadError>({ kind: "noUploadParams" })
     }
-
     const { uri, method, requiredHeaders } = uploadParams
 
-    const response = await fetch(uri, { method, headers: requiredHeaders, body: image.originalFile })
-
-    if (!response.ok) {
-      throw new Error("Failed to upload image")
-    }
-
-    return { ...image, url: uri }
-  } catch {
-    throw new Error("Failed to upload image")
-  }
+    return ResultAsync.fromPromise(
+      fetch(uri, { method, headers: requiredHeaders, body: fileImage.originalFile }),
+      (cause): UploadError => ({ kind: "uploadRequestFailed", cause }),
+    ).andThen(response => {
+      if (!response.ok) {
+        return errAsync<UploadedFileWithId, UploadError>({
+          kind: "uploadResponseNotOk",
+          status: response.status,
+          statusText: response.statusText,
+        })
+      }
+      return okAsync({ ...fileImage, url: uri })
+    })
+  })
 }

packages/intl/package.json

@@ -26,6 +26,7 @@
     "commander": "^12.0.0",
     "lilconfig": "^3.1.3",
     "lodash": "^4.17.23",
+    "neverthrow": "^8.2.0",
     "tslib": "^2.8.1",
     "zod": "^4.0.5"
   },

packages/intl/src/TranslationsServiceClient.ts

@@ -1,4 +1,13 @@
+import { ResultAsync } from "neverthrow"
 import type { ExtractedMessages } from "./formatjs"
+import type {
+  DownloadTermsError,
+  DownloadTranslationsError,
+  GetTranslationsInDefaultLanguageError,
+  RemoveTermsError,
+  UploadTermsError,
+  UploadTranslationsError,
+} from "./poeditor/POEditorError"
 
 export interface Term {
   term: string
@@ -10,10 +19,12 @@ export interface Term {
 type TermToRemove = Pick<Term, "context" | "term">
 
 export interface TranslationsServiceClient {
-  downloadTranslations(language: string): Promise<Record<string, string>>
-  uploadTerms(messages: ExtractedMessages): Promise<void>
-  uploadTranslations(messages: ExtractedMessages, language: string): Promise<void>
-  downloadTerms(): Promise<Term[]>
-  removeTerms(terms: TermToRemove[]): Promise<void>
-  getTranslationsInDefaultLanguage(terms: Term[]): Promise<{ term: string; translation: string }[]>
+  downloadTranslations(language: string): ResultAsync<Record<string, string>, DownloadTranslationsError>
+  uploadTerms(messages: ExtractedMessages): ResultAsync<void, UploadTermsError>
+  uploadTranslations(messages: ExtractedMessages, language: string): ResultAsync<void, UploadTranslationsError>
+  downloadTerms(): ResultAsync<Term[], DownloadTermsError>
+  removeTerms(terms: TermToRemove[]): ResultAsync<void, RemoveTermsError>
+  getTranslationsInDefaultLanguage(
+    terms: Term[],
+  ): ResultAsync<{ term: string; translation: string }[], GetTranslationsInDefaultLanguageError>
 }