[issues/619] Migrate to combined ExtensionError type#658
Conversation
|
Warning Review limit reached
More reviews will be available in 25 minutes and 57 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
The PR introduces changes to the command behavior by modifying how binding results are handled, specifically changing from 'Result' to 'ExtensionResult'. This affects the output of the binding commands, which is user-visible. Suggested test cases:
Generated by QA Gap Check (GPT-4o-mini via GitHub Models) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The extension naturally produces two error shapes: `RangeLinkError` from `rangelink-core-ts` and `RangeLinkExtensionError` from the extension itself. Define a combined `ExtensionError = RangeLinkError | RangeLinkExtensionError` union, widen `ExtensionResult<T>` to carry it, and let boundary functions pass core errors through instead of wrapping them. No behavior change at callers. - Add `ExtensionError` union type in `src/types/ExtensionError.ts`. - Widen `ExtensionResult<T>` and `ExtensionResult.err()` to accept `ExtensionError` rather than only `RangeLinkExtensionError`. Existing call sites are source-compatible because `RangeLinkExtensionError` is assignable to the union. - Simplify `generateLinkFromSelections`: pass `formatLink` errors through with `ExtensionResult.err(result.error)` instead of wrapping. Drop the now-unused `GENERATE_LINK_FORMAT_FAILED` code from `RangeLinkExtensionErrorCodes`. - Fix two mock setups (`SendRouter.test.ts`, `terminalInsertFactory.test.ts`) that were still using `RangeLinkError` to simulate `ClipboardService`, which moved to `RangeLinkExtensionError` in #609. Update the two `formatLink` error tests in `generateLinkFromSelections.test.ts` to assert on the pass-through core error. - Side-quest cleanup in the same diff: add `spyOnFormatLink` and `spyOnFindLinksInText` helpers, then remove the `import * as rangelinkCore from 'rangelink-core-ts'` namespace imports from three test files that only used them to drive `jest.spyOn`. This isolates the namespace-import pattern to the helper layer, matching the existing `spyOn*` convention. - `RangeLinkExtensionErrorCodes` is already a strict superset of `RangeLinkErrorCodes`. The combined union does not need any code-table changes. - The project already had a helper convention (`spyOnToInputSelection.ts`, `spyOnFormatMessage.ts`, etc.) for the `import * as` + `jest.spyOn` pattern. Following it was strictly better than the `jest.mock()` alternative that was first considered. - [x] All existing Jest tests pass (118 suites, 2060 tests, coverage 98.26% / 95.84% / 95.24% / 98.28%). - [x] Prettier and ESLint clean across the workspace. - [ ] `test:release:automated` will be exercised by CI. - Closes #619
✅ CI / Integration Tests (with overrides) — run summaryDuration: 0m 57s QA TC IDs: 2 exercised across 0 features Report: View run & artifacts Reproduce locally: Feature breakdown
|
✅ CI / Integration Tests (automated) — run summaryDuration: 11m 7s QA TC IDs: 163 exercised across 0 features Report: View run & artifacts Reproduce locally: Feature breakdown
|
✅ CI / Integration Tests (with extensions) — run summaryDuration: 12m 59s Unit tests: Ran in separate Test & Validate job QA TC IDs: 195 exercised across 0 features Report: View run & artifacts Reproduce locally: Feature breakdown
|
1 participant
Summary
The extension naturally produces two error shapes:
RangeLinkErrorfromrangelink-core-tsandRangeLinkExtensionErrorfrom the extension itself. Define a combinedExtensionError = RangeLinkError | RangeLinkExtensionErrorunion, widenExtensionResult<T>to carry it, and let boundary functions pass core errors through instead of wrapping them. No behavior change at callers.Changes
ExtensionErrorunion type insrc/types/ExtensionError.ts.ExtensionResult<T>andExtensionResult.err()to acceptExtensionErrorrather than onlyRangeLinkExtensionError. Existing call sites are source-compatible becauseRangeLinkExtensionErroris assignable to the union.generateLinkFromSelections: passformatLinkerrors through withExtensionResult.err(result.error)instead of wrapping. Drop the now-unusedGENERATE_LINK_FORMAT_FAILEDcode fromRangeLinkExtensionErrorCodes.SendRouter.test.ts,terminalInsertFactory.test.ts) that were still usingRangeLinkErrorto simulateClipboardService, which moved toRangeLinkExtensionErrorin Extract ContextKeyService from PasteDestinationManager #609. Update the twoformatLinkerror tests ingenerateLinkFromSelections.test.tsto assert on the pass-through core error.spyOnFormatLinkandspyOnFindLinksInTexthelpers, then remove theimport * as rangelinkCore from 'rangelink-core-ts'namespace imports from three test files that only used them to drivejest.spyOn. This isolates the namespace-import pattern to the helper layer, matching the existingspyOn*convention.Key Discoveries
RangeLinkExtensionErrorCodesis already a strict superset ofRangeLinkErrorCodes. The combined union does not need any code-table changes.spyOnToInputSelection.ts,spyOnFormatMessage.ts, etc.) for theimport * as+jest.spyOnpattern. Following it was strictly better than thejest.mock()alternative that was first considered.Test Plan
test:release:automatedwill be exercised by CI.Related