fix: migrates another part of indexer to strict types#2686
fix: migrates another part of indexer to strict types#2686
Conversation
📝 WalkthroughWalkthroughThis PR introduces TypeScript type safety improvements across the indexer module by adding optional chaining guards for GRPC data access, updating function signatures to handle undefined inputs, and expanding strict type checking configuration. Input validation is also enhanced for identifier processing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
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 (3)
apps/indexer/src/shared/utils/files.ts (1)
38-53:⚠️ Potential issue | 🟠 MajorUse
LoggerServiceinstead ofconsole.errorfor logging errors.Both
parseSizeStr(line 51) andparseDecimalKubernetesString(line 79) useconsole.errorwhich violates the logging standard. Replace withLoggerService.forContext()or remove the redundant logging before the error is thrown.apps/indexer/src/db/keybaseProvider.ts (1)
14-35:⚠️ Potential issue | 🟠 MajorReplace
console.*logging withLoggerService.The code uses
console.warn(line 15),console.log(line 19), andconsole.error(line 34) for logging. Per the coding guidelines, useLoggerServiceinstead to comply with the project's standardized logging approach.apps/indexer/src/providers/statusEndpointHandlers/grpc.ts (1)
13-38:⚠️ Potential issue | 🔴 CriticalGuard
.map()and.includes()calls against undefined values.The optional chaining on
data.cluster?.inventory?.cluster?.nodes(line 15) and...storage(line 34) returns undefined if any intermediate property is falsy, and calling.map()directly on undefined will throw a TypeError. Similarly,node.capabilities?.storageClasses.includes()on lines 71–73 uses incomplete optional chaining—ifnode.capabilitiesis undefined, the.includes()call will throw. Additionally, the double cast on line 90 (as unknown as ProviderStatusInfo["storage"]) can be avoided by properly typingstorageat declaration (line 34). UpdategetAvailableResourcesto acceptNodeResources | undefinedto remove the non-null assertion on line 16.Proposed fix
- const availableResources = data.cluster?.inventory?.cluster?.nodes - .map(x => getAvailableResources(x.resources!)) + const nodes = data.cluster?.inventory?.cluster?.nodes ?? []; + const availableResources = nodes + .map(x => getAvailableResources(x.resources)) .reduce( (prev, next) => ({ cpu: prev.cpu + next.cpu, @@ - const storage = data.cluster?.inventory?.cluster?.storage.map(storage => ({ + const storage: ProviderStatusInfo["storage"] = (data.cluster?.inventory?.cluster?.storage ?? []).map(storage => ({ class: storage?.info?.class, allocatable: parseSizeStr(storage?.quantity?.allocatable?.string), allocated: parseSizeStr(storage?.quantity?.allocated?.string) })); @@ - storage: storage as unknown as ProviderStatusInfo["storage"] + storage }; } @@ -function getAvailableResources(resources: NodeResources) { +function getAvailableResources(resources: NodeResources | undefined) { @@ - capabilitiesStorageHDD: !!node.capabilities?.storageClasses.includes("beta1"), - capabilitiesStorageSSD: !!node.capabilities?.storageClasses.includes("beta2"), - capabilitiesStorageNVME: !!node.capabilities?.storageClasses.includes("beta3"), + capabilitiesStorageHDD: !!node.capabilities?.storageClasses?.includes("beta1"), + capabilitiesStorageSSD: !!node.capabilities?.storageClasses?.includes("beta2"), + capabilitiesStorageNVME: !!node.capabilities?.storageClasses?.includes("beta3"),
🤖 Fix all issues with AI agents
In `@apps/indexer/src/providers/statusEndpointHandlers/grpc.ts`:
- Around line 71-73: The three capability booleans (capabilitiesStorageHDD,
capabilitiesStorageSSD, capabilitiesStorageNVME) call
node.capabilities?.storageClasses.includes(...), which can throw if
storageClasses is undefined; update each to safely check with optional chaining
and a default (e.g., node.capabilities?.storageClasses?.includes(...) ?? false)
so the expression returns false instead of throwing when storageClasses is
missing. Locate these symbols in grpc.ts and apply the change to all three
lines.
🧹 Nitpick comments (1)
apps/indexer/tsconfig.build.json (1)
2-8: ConfirmnoImplicitAnyoverride is intended withstrictenabled.With
"strict": true, the explicit"noImplicitAny": falsedisables a core strictness check. If the goal is full strictness, consider removing the override or setting it totrue; otherwise please confirm it’s intentional.Suggested adjustment
- "noImplicitAny": false, + "noImplicitAny": true,
c38559f to
84a84b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/indexer/src/providers/statusEndpointHandlers/grpc.ts`:
- Around line 15-16: The call site uses a non-null assertion on x.resources when
building availableResources; change getAvailableResources to accept resources?:
NodeResources (or the appropriate type) and handle undefined internally (reusing
parseNodeResources behavior), then remove the non-null assertion in the map
(i.e., call getAvailableResources(x.resources)); update the
getAvailableResources signature and any callers to accept undefined so no
runtime error occurs when resources is missing.
84a84b8 to
51a7b88
Compare
Why
to improve reliability on indexer
Summary by CodeRabbit
Bug Fixes
Refactor