fix(core): defensive symbol/type handling in MethodTransformer (#1382)#1448
fix(core): defensive symbol/type handling in MethodTransformer (#1382)#1448wildduck2 wants to merge 3 commits into
Conversation
Fixes samchon#1382. The get_name() and escape_promise() helpers used non-null assertions on values that the TypeScript API documents as potentially undefined. When a method return type resolves to a symbol without declarations (e.g. via import() type expressions on older TS versions), these helpers crashed with: TypeError: Cannot read properties of undefined (reading '0') or threw: Error on ImportAnalyzer.analyze(): invalid promise type. Replace the unsafe assertions with optional chaining and safe fallbacks, matching the pattern already used in ParameterDecoratorTransformer.ts:132-134 and DtoAnalyzer.ts:248-251 within the same codebase.
Adds a new test feature that exercises the symbol and type paths previously guarded by non-null assertions in MethodTransformer. Covers: - inline/anonymous object return types wrapped in Promise - intersection of named interfaces - ReturnType<typeof fn> utility types - infer keyword in conditional types - generic envelope types - type aliases over interfaces - import() type expressions in TypedRoute generic arguments Follows the structure of existing clone-based features under tests/test-sdk/features/ and is gitignored for generated output.
|
Note on the investigation path and why the PR description is long. The issue had two reports (the original bug and The fix itself is small and self-contained in |
|
Closing to move this to my own fork first while I finalize things. Will reopen when ready. |
|
Note on why this only reproduces on older TypeScript versions ( The symbol returned by On TLDR: on TS |
Coverage gap and follow-up questionsI want to be upfront about what the added test feature actually verifies and what it does not, and get your input on how far you want this PR to go. What the current tests verifyThe test feature under What the tests do not verifyOn the workspace TS version ( The only environment where the fallback branch actually fires is the original reporter's toolchain: Suggested optionsOrdered from smallest to largest change:
Let me know which direction you prefer and I will follow through. |
There was a problem hiding this comment.
Pull request overview
Hardens MethodTransformer against TypeScript API edge cases where symbols have no declarations and where Promise type arguments can’t be reliably extracted, preventing nestia swagger generation crashes reported in #1382.
Changes:
- Make
get_name()andescape_promise()defensive to avoid runtime crashes on transient/synthesized TypeScript symbols and atypical generic-argument extraction results. - Add a new test-sdk feature project (
method-transformer-defensive) exercising inline/utility/conditional/import() return-type shapes. - Ignore generated structures output for the new feature in test-sdk
.gitignore.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/transformers/MethodTransformer.ts | Adds defensive guards around symbol declaration access and Promise unwrapping. |
| tests/test-sdk/features/method-transformer-defensive/tsconfig.json | Test feature TS config mirroring other feature layouts. |
| tests/test-sdk/features/method-transformer-defensive/nestia.config.ts | Test feature Nestia config to generate SDK/swagger for the repro shapes. |
| tests/test-sdk/features/method-transformer-defensive/src/controllers/InlineReturnController.ts | Adds Promise-wrapped inline/intersection/ReturnType/infer return shapes. |
| tests/test-sdk/features/method-transformer-defensive/src/controllers/ImportTypeController.ts | Adds import() type-expression and alias/envelope shapes for coverage. |
| tests/test-sdk/.gitignore | Ignores generated structures for the new feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Note: this PR only partially addresses the broader issue discussed in #1393. It hardens |
Fixes #1382.
Summary
Two helpers in
packages/core/src/transformers/MethodTransformer.tsrelied on non-null assertions over values the TypeScript API documents as potentiallyundefined. When a method return type resolves to a symbol without declarations, the transformer crashed with:Or threw:
This PR makes both helpers defensive. Generation now succeeds and produces the correct output type on the repro.
Root cause
Two functions in
MethodTransformer.tsassumed the TypeScript API always returns populated arrays:get_name(symbol)dereferencedsymbol.getDeclarations()![0]!.parent. The TypeScript API spec saysgetDeclarations()can returnundefinedor an empty array for transient/synthesized symbols (symbols created fromimport()type expressions, certain utility type resolutions, and some inferred types). Hitting that path on TypeScript 5.7.x produces theTypeErrorabove.escape_promise(type)calledchecker.getTypeArguments()and threw a hard error unless exactly one type argument was returned. Conditional types, already-resolved generics, and certainimport()type expressions can legally return zero arguments.The same package already uses the safe pattern in
ParameterDecoratorTransformer.tsandpackages/sdk/src/analyses/DtoAnalyzer.ts. This change alignsMethodTransformerwith that pattern.Fix
get_name(): use optional chaining, fall back tosymbol.escapedNamewhen no parent declaration is available.escape_promise(): wrap the call intry/catch, return the original type when type arguments cannot be extracted or length is not1.Both changes preserve existing behavior on the happy path. When a symbol does have declarations and
Promisehas one type argument, the output is identical. The fallbacks only take over in the edge cases that previously crashed.How the bug was narrowed down
Read the existing report on @nestia/core MethodTransformer crashes on inline/utility Promise return types ("get_name" undefined / "invalid promise type") #1382 and its stack trace pointing at
MethodTransformer.ts:92.Cloned the reporter's public repro (
Neuron-Grid/Mycelia-API) to see the exact patterns used in the failing controllers.Built a minimal demonstration project that mirrors the pattern:
@TypedRoute.Get<import("...").Envelope<import("...").Dto>>(""), with a controller return type using a type alias.Confirmed the crash on
@nestia/core@8.0.5withtypia@9.7.2andtypescript@5.7.2.Added
console.logto the compiled helpers in the installed@nestia/coreto capture which symbol the crash was on. The log showed:confirming the symbol had no declarations before the unsafe dereference.
Applied the defensive fix to the installed compiled JS to verify the fix stops the crash and restores correct output type resolution (SDK then produced the correct typed
Outputinstead of crashing).Ported the fix to the workspace source in
packages/core/src/transformers/MethodTransformer.ts.On current
master(typia@12,typescript@5.9.3) the crash does not reproduce even without the fix, because upstream TypeScript and typia resolve those symbols differently. The unsafe code path is still latent, so the defensive changes harden it for users on older toolchains and against future regressions.Tests
Adds
tests/test-sdk/features/method-transformer-defensive/following the same layout as existing clone-based features. It covers:PromiseReturnType<typeof fn>utility typesinferkeyword in conditional typesimport()type expressions in a@TypedRoutegeneric argumentRun the standard test automation:
pnpm install npm run build npm run testReproducing the original crash
The repro cannot be committed because it intentionally pins older dependencies. Use the config below in a clean directory to reproduce the original failure on the pre-fix code.
package.json:{ "private": true, "name": "nestia-1382-repro", "scripts": { "openapi:gen": "nestia swagger", "prepare": "ts-patch install && typia patch" }, "dependencies": { "@nestia/core": "8.0.5", "@nestia/fetcher": "8.0.5", "@nestjs/common": "11.1.6", "@nestjs/core": "11.1.6", "@nestjs/platform-express": "11.1.6", "reflect-metadata": "0.2.2", "typia": "9.7.2" }, "devDependencies": { "@nestia/sdk": "8.0.5", "@types/node": "24.3.1", "nestia": "8.0.5", "ts-node": "10.9.2", "ts-patch": "3.3.0", "tsconfig-paths": "4.2.0", "typescript": "5.7.2", "typescript-transform-paths": "3.5.6" } }tsconfig.json:{ "compilerOptions": { "module": "CommonJS", "moduleResolution": "node", "target": "ES2023", "lib": ["ES2023"], "strict": true, "experimentalDecorators": true, "emitDecoratorMetadata": true, "esModuleInterop": true, "skipLibCheck": true, "rootDir": "src", "outDir": "./dist", "baseUrl": "./", "paths": { "@/*": ["src/*"] }, "plugins": [ { "transform": "typia/lib/transform", "resolvePathAliases": true, "tsConfig": "./tsconfig.json" }, { "transform": "@nestia/core/lib/transform" }, { "transform": "@nestia/sdk/lib/transform" } ] }, "include": [ "src/app.controller.ts", "src/app.service.ts", "src/common/dto/greeting.dto.ts", "src/common/utils/response.util.ts" ] }nestia.config.ts:src/app.service.ts:src/common/dto/greeting.dto.ts:src/common/utils/response.util.ts:src/app.controller.ts:Commands:
Without the fix this crashes with the
TypeErrorabove. With the fix swagger generation completes and the SDKOutputtype resolves to the expected envelope type.Notes on docs
CONTRIBUTING.mdalready states that new code should ship with a test demonstration project. The test undertests/test-sdk/features/method-transformer-defensive/satisfies that requirement. No additional doc updates appear to be required for this change. If aCHANGELOGentry or similar is wanted, happy to add one on request.