fixing eslint issues #1480
fixing eslint issues #1480
Conversation
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
WalkthroughStandardizes quoting/semicolon styles and formatting across many files, refactors tests, adds file helper functions (createFile, existence check), expands verification DTO properties, adds getContextId to context service, tightens a delay parameter type, and removes an Elasticsearch transport error listener in logger. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant FileHelper as file-operation.helper
participant FS as File System
Caller->>FileHelper: createFile(path, fileName, data)
activate FileHelper
FileHelper->>FS: checkIfFileOrDirectoryExists(path)
alt directory missing
FileHelper->>FS: mkdir(path, { recursive: true })
else directory exists
note right of FileHelper: proceed
end
FileHelper->>FS: writeFile(path/fileName, data)
deactivate FileHelper
FileHelper-->>Caller: Promise<void>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/context/src/nestjsClsContextStorageService.ts (1)
17-19: Align getContextId return type with interface
getContextId()innestjsClsContextStorageService.tsreturnsstring | undefined, butContextStorageService(incontextStorageService.interface.ts) declaresgetContextId(): string, causing compile errors. Either:
- Narrow implementation to
string:- public getContextId(): string | undefined { - return this.cls.getId(); - } + public getContextId(): string { + const id = this.cls.getId(); + if (!id) throw new Error('Context ID is not set'); + return id; + }- Or change the interface to
getContextId(): string | undefinedand update all callers to handleundefined.
🧹 Nitpick comments (7)
apps/issuance/src/issuance.service.ts (1)
1504-1506: Tighten the delay return typeWhile you’re here, please narrow the signature to
Promise<void>—the promise never resolves with a value, so the currentPromise<unknown>is overly loose. This keeps downstream usage strongly typed.apps/utility/src/utilities.repository.ts (4)
3-4: Remove eslint-disable; alias Prisma type instead.Avoid disabling camelcase by aliasing the Prisma type.
-// eslint-disable-next-line camelcase -import { shortening_url } from '@prisma/client'; +import type { shortening_url as ShorteningUrl } from '@prisma/client';
13-20: Type the payload and return value; align with Prisma result.Return the concrete Prisma model and type the payload fields.
- async saveShorteningUrl(payload): Promise<object> { + async saveShorteningUrl(payload: SaveShorteningUrlPayload): Promise<ShorteningUrl> { try { const { referenceId, invitationPayload } = payload; const storeShorteningUrl = await this.prisma.shortening_url.upsert({ where: { referenceId }, update: { invitationPayload }, create: { referenceId, invitationPayload } });Add near the top of this file:
interface SaveShorteningUrlPayload { referenceId: string; // Adjust as needed if JSON type is available from Prisma (e.g., Prisma.JsonValue) invitationPayload: unknown; }
22-26: Improve error logging detail.Log message and stack for better diagnostics.
- this.logger.error(`Error in saveShorteningUrl: ${error} `); + this.logger.error( + `Error in saveShorteningUrl: ${error?.message ?? error}`, + (error as Error)?.stack + );
42-43: Consistent error logging.Align with earlier logging improvement.
- this.logger.error(`Error in getShorteningUrl: ${error} `); + this.logger.error( + `Error in getShorteningUrl: ${error?.message ?? error}`, + (error as Error)?.stack + );apps/connection/src/interfaces/connection.interfaces.ts (1)
127-134: Use imported IUserRequest instead of local IUserRequestInterface
Local IUserRequestInterface duplicates the shape of IUserRequest (userId, email), so there’s no immediate assignability break—but maintaining one shared type prevents divergence. Replace IUserRequestInterface in IConnectionSearchCriteria, IReceiveInvitationByUrlOrg, and IReceiveInvitationByOrg with the imported IUserRequest.apps/api-gateway/src/verification/dto/webhook-proof.dto.ts (1)
13-13: Consider using specific types instead ofobjectfor better type safety.The
metadataandproofDataproperties are typed asobject, which provides minimal type safety. Consider defining specific interfaces or usingRecord<string, unknown>if the structure is truly dynamic.For example:
interface PresentationMetadata { // Define known properties [key: string]: unknown; // Allow additional properties if needed } @ApiPropertyOptional() @IsOptional() metadata?: PresentationMetadata;Similarly for
proofData, define the expected structure if known, or use a more specific type likeRecord<string, unknown>to maintain some type safety while allowing flexibility.Also applies to: 73-73
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/agent-service/test/app.e2e-spec.ts(1 hunks)apps/api-gateway/src/cloud-wallet/enums/connections.enum.ts(1 hunks)apps/api-gateway/src/connection/enums/connections.enum.ts(1 hunks)apps/api-gateway/src/connection/interfaces/index.ts(3 hunks)apps/api-gateway/src/dtos/credential-offer.dto.ts(1 hunks)apps/api-gateway/src/dtos/credential-send-offer.dto.ts(1 hunks)apps/api-gateway/src/helper-files/file-operation.helper.ts(1 hunks)apps/api-gateway/src/verification/dto/webhook-proof.dto.ts(1 hunks)apps/api-gateway/src/verification/interfaces/verification.interface.ts(1 hunks)apps/connection/src/interfaces/connection.interfaces.ts(6 hunks)apps/issuance/src/issuance.service.ts(1 hunks)apps/user/test/app.e2e-spec.ts(1 hunks)apps/utility/src/utilities.repository.ts(1 hunks)libs/context/src/nestjsClsContextStorageService.ts(2 hunks)libs/logger/src/winstonLogger.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
libs/context/src/nestjsClsContextStorageService.ts (1)
libs/context/src/contextStorageService.interface.ts (1)
ContextStorageService(3-8)
apps/api-gateway/src/connection/interfaces/index.ts (5)
apps/connection/src/interfaces/connection.interfaces.ts (1)
IOrgAgentInterface(50-58)apps/agent-service/src/interface/agent-service.interface.ts (1)
IOrgAgentInterface(275-283)apps/ledger/src/credential-definition/interfaces/index.ts (1)
IOrgAgentInterface(34-42)apps/api-gateway/src/schema/interfaces/index.ts (1)
IOrgAgentInterface(34-42)apps/api-gateway/src/credential-definition/interfaces/index.ts (1)
IOrgAgentInterface(33-41)
apps/api-gateway/src/verification/interfaces/verification.interface.ts (1)
apps/api-gateway/src/connection/interfaces/index.ts (1)
IUserRequestInterface(3-15)
apps/connection/src/interfaces/connection.interfaces.ts (2)
apps/api-gateway/src/connection/interfaces/index.ts (1)
IUserRequestInterface(3-15)apps/agent-service/src/interface/agent-service.interface.ts (5)
IUserRequestInterface(237-250)IReceiveInvitationUrl(500-502)IReceiveInvite(490-498)IReceiveInvitation(525-527)ICreateConnectionInvitation(577-591)
libs/logger/src/winstonLogger.ts (2)
libs/logger/src/logger.interface.ts (1)
Logger(6-20)libs/logger/src/log.ts (1)
LogData(17-26)
apps/utility/src/utilities.repository.ts (5)
apps/api-gateway/src/platform/platform.service.ts (1)
Injectable(11-46)apps/api-gateway/src/utilities/utilities.service.ts (1)
Injectable(7-25)apps/connection/src/connection.service.ts (2)
Injectable(36-745)storeShorteningUrl(93-100)apps/connection/src/connection.repository.ts (2)
Injectable(11-397)storeShorteningUrl(201-213)apps/utility/src/utilities.service.ts (1)
Injectable(8-63)
🔇 Additional comments (8)
apps/api-gateway/src/verification/interfaces/verification.interface.ts (1)
4-92: Consistent interface formatting looks good.Thanks for standardizing the property terminations here; it keeps the interface definitions aligned with the rest of the codebase.
apps/api-gateway/src/cloud-wallet/enums/connections.enum.ts (1)
2-4: LGTM — quote normalization only.apps/connection/src/interfaces/connection.interfaces.ts (2)
19-20: LGTM — adding optionalinvitationDidis safe and useful.Also applies to: 269-270
apps/api-gateway/src/helper-files/file-operation.helper.ts (2)
16-20: LGTM!The
getFilefunction correctly uses promisified async operations and handles both encoded and raw buffer reads appropriately.
22-26: LGTM!The
deleteFilefunction correctly uses promisified async operations.apps/api-gateway/src/connection/interfaces/index.ts (1)
29-29: LGTM! Formatting changes align with project conventions.The semicolon addition and quote style normalization improve code consistency without altering any behavior.
Also applies to: 72-73, 115-115
apps/api-gateway/src/verification/dto/webhook-proof.dto.ts (1)
11-82: Verify that all properties should be optional.All properties in this DTO are decorated with
@IsOptional(). If any of these properties are required for webhook processing (e.g.,id,state,connectionId), consider using@IsString(),@IsBoolean(), etc., without@IsOptional()to enforce validation.For example, if
idis always present:@ApiPropertyOptional() -@IsOptional() +@IsString() id: string;Review the webhook payload structure to determine which fields are guaranteed to be present and adjust decorators accordingly.
Based on learnings.
| @ApiProperty({ example: { protocolVersion: 'v1' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid protocol-version' }) | ||
| @IsString({ message: 'protocol-version should be string' }) | ||
| protocolVersion: string; | ||
|
|
||
| @ApiProperty({ example: { 'protocolVersion': 'v1' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid protocol-version' }) | ||
| @IsString({ message: 'protocol-version should be string' }) | ||
| protocolVersion: string; | ||
|
|
||
| @ApiProperty({ example: { 'attributes': [{ 'value': 'string', 'name': 'string' }] } }) | ||
| @IsNotEmpty({ message: 'Please provide valid attributes' }) | ||
| @IsArray({ message: 'attributes should be array' }) | ||
| attributes: attributeValue[]; | ||
| @ApiProperty({ example: { attributes: [{ value: 'string', name: 'string' }] } }) | ||
| @IsNotEmpty({ message: 'Please provide valid attributes' }) | ||
| @IsArray({ message: 'attributes should be array' }) | ||
| attributes: attributeValue[]; | ||
|
|
||
| @ApiProperty({ example: { 'credentialDefinitionId': 'string' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid credentialDefinitionId' }) | ||
| @IsString({ message: 'credentialDefinitionId should be string' }) | ||
| credentialDefinitionId: string; | ||
| @ApiProperty({ example: { credentialDefinitionId: 'string' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid credentialDefinitionId' }) | ||
| @IsString({ message: 'credentialDefinitionId should be string' }) | ||
| credentialDefinitionId: string; | ||
|
|
||
| @ApiProperty({ example: { autoAcceptCredential: 'always' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid autoAcceptCredential' }) | ||
| @IsString({ message: 'autoAcceptCredential should be string' }) | ||
| autoAcceptCredential: string; | ||
| @ApiProperty({ example: { autoAcceptCredential: 'always' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid autoAcceptCredential' }) | ||
| @IsString({ message: 'autoAcceptCredential should be string' }) | ||
| autoAcceptCredential: string; | ||
|
|
||
| @ApiProperty({ example: { comment: 'string' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid comment' }) | ||
| @IsString({ message: 'comment should be string' }) | ||
| comment: string; | ||
| @ApiProperty({ example: { comment: 'string' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid comment' }) | ||
| @IsString({ message: 'comment should be string' }) | ||
| comment: string; | ||
|
|
||
| @ApiProperty({ example: { connectionId: '3fa85f64-5717-4562-b3fc-2c963f66afa6' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid connection-id' }) | ||
| @IsString({ message: 'Connection-id should be string' }) | ||
| connectionId: string; | ||
| @ApiProperty({ example: { connectionId: '3fa85f64-5717-4562-b3fc-2c963f66afa6' } }) | ||
| @IsNotEmpty({ message: 'Please provide valid connection-id' }) | ||
| @IsString({ message: 'Connection-id should be string' }) | ||
| connectionId: string; |
There was a problem hiding this comment.
Swagger examples are incorrectly nested; use field-value examples.
Examples should represent the property value directly, not { propertyName: value }. Fix to avoid misleading API docs.
- @ApiProperty({ example: { protocolVersion: 'v1' } })
+ @ApiProperty({ example: 'v1' })
@IsNotEmpty({ message: 'Please provide valid protocol-version' })
@IsString({ message: 'protocol-version should be string' })
protocolVersion: string;
- @ApiProperty({ example: { attributes: [{ value: 'string', name: 'string' }] } })
+ @ApiProperty({ example: [{ name: 'firstName', value: 'Alice' }] })
@IsNotEmpty({ message: 'Please provide valid attributes' })
@IsArray({ message: 'attributes should be array' })
attributes: attributeValue[];
- @ApiProperty({ example: { credentialDefinitionId: 'string' } })
+ @ApiProperty({ example: 'creddef:XYZ' })
@IsNotEmpty({ message: 'Please provide valid credentialDefinitionId' })
@IsString({ message: 'credentialDefinitionId should be string' })
credentialDefinitionId: string;
- @ApiProperty({ example: { autoAcceptCredential: 'always' } })
+ @ApiProperty({ example: 'always' })
@IsNotEmpty({ message: 'Please provide valid autoAcceptCredential' })
@IsString({ message: 'autoAcceptCredential should be string' })
autoAcceptCredential: string;
- @ApiProperty({ example: { comment: 'string' } })
+ @ApiProperty({ example: 'Issuing employment credential' })
@IsNotEmpty({ message: 'Please provide valid comment' })
@IsString({ message: 'comment should be string' })
comment: string;
- @ApiProperty({ example: { connectionId: '3fa85f64-5717-4562-b3fc-2c963f66afa6' } })
+ @ApiProperty({ example: '3fa85f64-5717-4562-b3fc-2c963f66afa6' })
@IsNotEmpty({ message: 'Please provide valid connection-id' })
@IsString({ message: 'Connection-id should be string' })
connectionId: string;Optional:
- Rename
attributeValuetoAttributeValue(PascalCase for interfaces). - Use
@IsUUID()forconnectionId.
🤖 Prompt for AI Agents
In apps/api-gateway/src/dtos/credential-offer.dto.ts around lines 10 to 38, the
@ApiProperty examples are incorrectly wrapped as objects (e.g. {
protocolVersion: 'v1' }) which misleads Swagger—change each example to the
direct property value (e.g. example: 'v1' or example: [{ value: 'string', name:
'string' }]); also follow the optional suggestions: rename the attributeValue
type to PascalCase AttributeValue everywhere and replace the connectionId
validation with @IsUUID() (or add @IsUUID() alongside @IsString()) to ensure
proper UUID validation.
| const writeFile = promisify(fs.writeFile); | ||
| return fs.writeFileSync(`${path}/${fileName}`, data, 'utf8'); |
There was a problem hiding this comment.
Remove unused promisified writeFile and use async write or make function sync.
The function promisifies fs.writeFile but never uses it, then calls the synchronous fs.writeFileSync. This is inconsistent and the unused variable triggers an ESLint warning.
Choose one approach:
Option 1 (async): Use the promisified version consistently:
export const createFile = async (path: string, fileName: string, data: string): Promise<void> => {
// eslint-disable-next-line @typescript-eslint/no-use-before-define
if (!checkIfFileOrDirectoryExists(path)) {
fs.mkdirSync(path);
}
- // eslint-disable-next-line @typescript-eslint/no-unused-vars
const writeFile = promisify(fs.writeFile);
- return fs.writeFileSync(`${path}/${fileName}`, data, 'utf8');
+ return writeFile(`${path}/${fileName}`, data, 'utf8');
};Option 2 (sync): Remove async and promisify:
-export const createFile = async (path: string, fileName: string, data: string): Promise<void> => {
+export const createFile = (path: string, fileName: string, data: string): void => {
// eslint-disable-next-line @typescript-eslint/no-use-before-define
if (!checkIfFileOrDirectoryExists(path)) {
fs.mkdirSync(path);
}
- // eslint-disable-next-line @typescript-eslint/no-unused-vars
- const writeFile = promisify(fs.writeFile);
- return fs.writeFileSync(`${path}/${fileName}`, data, 'utf8');
+ fs.writeFileSync(`${path}/${fileName}`, data, 'utf8');
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const writeFile = promisify(fs.writeFile); | |
| return fs.writeFileSync(`${path}/${fileName}`, data, 'utf8'); | |
| export const createFile = async (path: string, fileName: string, data: string): Promise<void> => { | |
| // eslint-disable-next-line @typescript-eslint/no-use-before-define | |
| if (!checkIfFileOrDirectoryExists(path)) { | |
| fs.mkdirSync(path); | |
| } | |
| const writeFile = promisify(fs.writeFile); | |
| return writeFile(`${path}/${fileName}`, data, 'utf8'); | |
| }; |
| const writeFile = promisify(fs.writeFile); | |
| return fs.writeFileSync(`${path}/${fileName}`, data, 'utf8'); | |
| export const createFile = (path: string, fileName: string, data: string): void => { | |
| // eslint-disable-next-line @typescript-eslint/no-use-before-define | |
| if (!checkIfFileOrDirectoryExists(path)) { | |
| fs.mkdirSync(path); | |
| } | |
| fs.writeFileSync(`${path}/${fileName}`, data, 'utf8'); | |
| }; |
🤖 Prompt for AI Agents
In apps/api-gateway/src/helper-files/file-operation.helper.ts around lines
10-11, the code promisifies fs.writeFile to a writeFile constant but then calls
fs.writeFileSync, leaving an unused variable and inconsistent sync/async
behavior; fix by either removing the promisify and the unused writeFile constant
(keep the synchronous fs.writeFileSync and ensure the function is
documented/typed as sync) or convert the function to async: replace
fs.writeFileSync with await writeFile(pathWithFileName, data, 'utf8'), mark the
function async, and remove any unused sync import; also remove any unused eslint
warnings by deleting the unused variable or using the promisified call.
| export interface ICreateOutOfbandConnectionInvitation { | ||
| user: IUserRequestInterface, | ||
| createOutOfBandConnectionInvitation: ICreateConnectionInvitation, | ||
| } | ||
| user: IUserRequestInterface; | ||
| createOutOfBandConnectionInvitation: ICreateConnectionInvitation; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Inconsistent casing: ICreateOutOfbandConnectionInvitation.
Use OutOfBand (capital B) consistently to match related types (OutOfBandInvitation).
Proposed rename:
-export interface ICreateOutOfbandConnectionInvitation {
+export interface ICreateOutOfBandConnectionInvitation {
user: IUserRequestInterface;
createOutOfBandConnectionInvitation: ICreateConnectionInvitation;
}Search for references to update:
🏁 Script executed:
#!/bin/bash
rg -n -C2 -g '!**/node_modules/**' $'ICreateOutOfbandConnectionInvitation|ICreateOutOfBandConnectionInvitation'Length of output: 2117
Rename ICreateOutOfbandConnectionInvitation → ICreateOutOfBandConnectionInvitation
Update the interface declaration in apps/connection/src/interfaces/connection.interfaces.ts and all imports/usages in apps/connection/src/connection.controller.ts and apps/connection/src/connection.service.ts to the new name.
-export interface ICreateOutOfbandConnectionInvitation {
+export interface ICreateOutOfBandConnectionInvitation {
user: IUserRequestInterface;
createOutOfBandConnectionInvitation: ICreateConnectionInvitation;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface ICreateOutOfbandConnectionInvitation { | |
| user: IUserRequestInterface, | |
| createOutOfBandConnectionInvitation: ICreateConnectionInvitation, | |
| } | |
| \ No newline at end of file | |
| user: IUserRequestInterface; | |
| createOutOfBandConnectionInvitation: ICreateConnectionInvitation; | |
| } | |
| export interface ICreateOutOfBandConnectionInvitation { | |
| user: IUserRequestInterface; | |
| createOutOfBandConnectionInvitation: ICreateConnectionInvitation; | |
| } |
🤖 Prompt for AI Agents
In apps/connection/src/interfaces/connection.interfaces.ts around lines 291-294
rename the interface ICreateOutOfbandConnectionInvitation to
ICreateOutOfBandConnectionInvitation (capital B in "Band"); then update all
references and imports to that interface in
apps/connection/src/connection.controller.ts and
apps/connection/src/connection.service.ts to use the new name, adjusting any
export/import statements and type annotations to match exactly.
| // eslint-disable-next-line camelcase | ||
| async getShorteningUrl(referenceId): Promise<shortening_url> { | ||
| try { | ||
| const storeShorteningUrl = await this.prisma.shortening_url.findUnique({ | ||
| where: { | ||
| referenceId | ||
| } | ||
| }); | ||
|
|
||
| this.logger.log(`[getShorteningUrl] - shortening url details ${referenceId}`); | ||
| return storeShorteningUrl; | ||
| } catch (error) { |
There was a problem hiding this comment.
🧩 Analysis chain
Nullability mismatch: findUnique may return null; type/handling needed.
findUnique can return null, but the method promises a non-null return and callers (e.g., apps/utility/src/utilities.service.ts:getShorteningUrl) dereference without checks, risking a runtime error.
Two options:
- Update types to reflect null and handle upstream.
- Or throw if not found.
- // eslint-disable-next-line camelcase
- async getShorteningUrl(referenceId): Promise<shortening_url> {
+ async getShorteningUrl(referenceId: string): Promise<ShorteningUrl | null> {
try {
- const storeShorteningUrl = await this.prisma.shortening_url.findUnique({
+ const shorteningUrl = await this.prisma.shortening_url.findUnique({
where: {
referenceId
}
});
-
- this.logger.log(`[getShorteningUrl] - shortening url details ${referenceId}`);
- return storeShorteningUrl;
+ this.logger.log(`[getShorteningUrl] - shortening url details ${referenceId}`);
+ return shorteningUrl;If you prefer throwing instead:
const shorteningUrl = await this.prisma.shortening_url.findUnique({ where: { referenceId } });
if (!shorteningUrl) {
this.logger.warn(`[getShorteningUrl] - not found for ${referenceId}`);
throw new Error('Shortening URL not found');
}
return shorteningUrl;Please verify/update the caller to handle null/exception accordingly.
🏁 Script executed:
#!/bin/bash
rg -n --type=ts "getShorteningUrl" -C3 apps/utility/src
rg -n --type=ts "findUnique" -C3 apps/utility/src/utilities.repository.tsLength of output: 3449
Handle potential null from Prisma.findUnique
findUnique can return null, but getShorteningUrl promises a non-null shortening_url, and the service immediately dereferences its fields. Either change the method’s return to Promise<ShorteningUrl | null> (and adjust the caller) or throw an error when no record is found.
🤖 Prompt for AI Agents
In apps/utility/src/utilities.repository.ts around lines 30 to 41, the method
getShorteningUrl calls prisma.findUnique which may return null but the function
signature promises a non-null shortening_url; update the implementation to
either (A) change the return type to Promise<shortening_url | null> and
propagate null handling to callers, or (B) keep the non-null signature and after
the findUnique check for null and throw a descriptive error (e.g.,
NotFoundError) so callers never receive null; also update any unit tests and
calling code to match the chosen contract.
| } | ||
| }; | ||
| esTransport = new Elasticsearch.ElasticsearchTransport(esTransportOpts); | ||
| } |
There was a problem hiding this comment.
Restore Elasticsearch transport error handler.
Dropping the 'error' listener leaves the transport’s EventEmitter without any handlers; when winston-elasticsearch emits an error, Node will throw (and can crash the process). Reintroduce a listener that logs via stderr (or another logger) to keep the process alive.
esTransport = new Elasticsearch.ElasticsearchTransport(esTransportOpts);
+ esTransport.on('error', (error: Error) => {
+ process.stderr.write(`Elasticsearch transport error: ${error.message}\n`);
+ });🤖 Prompt for AI Agents
In libs/logger/src/winstonLogger.ts around line 22, the code removed the 'error'
listener from the elasticsearch transport leaving its EventEmitter without
handlers which causes Node to throw on transport errors; reintroduce an 'error'
listener on the transport that logs the error to stderr (e.g., console.error or
process.stderr.write) or to a fallback logger so the error is recorded and the
process does not crash, and ensure the handler is simple and non-blocking to
avoid side effects.
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/api-gateway/src/helper-files/file-operation.helper.ts (1)
4-12: Address the unresolved async/sync inconsistency.This is the same issue flagged in the previous review. The function is declared
asyncbut uses synchronous operations (fs.writeFileSync), and the promisifiedwriteFileon line 10 remains unused despite the eslint-disable comment.Apply one of these fixes from the previous review:
Option 1 (async - recommended): Use the promisified version:
export const createFile = async (path: string, fileName: string, data: string): Promise<void> => { // eslint-disable-next-line @typescript-eslint/no-use-before-define if (!checkIfFileOrDirectoryExists(path)) { fs.mkdirSync(path, { recursive: true }); } - // eslint-disable-next-line @typescript-eslint/no-unused-vars const writeFile = promisify(fs.writeFile); - return fs.writeFileSync(`${path}/${fileName}`, data, 'utf8'); + await writeFile(`${path}/${fileName}`, data, 'utf8'); };Option 2 (sync): Remove async entirely:
-export const createFile = async (path: string, fileName: string, data: string): Promise<void> => { +export const createFile = (path: string, fileName: string, data: string): void => { // eslint-disable-next-line @typescript-eslint/no-use-before-define if (!checkIfFileOrDirectoryExists(path)) { fs.mkdirSync(path, { recursive: true }); } - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const writeFile = promisify(fs.writeFile); - return fs.writeFileSync(`${path}/${fileName}`, data, 'utf8'); + fs.writeFileSync(`${path}/${fileName}`, data, 'utf8'); };
🧹 Nitpick comments (2)
apps/api-gateway/src/helper-files/file-operation.helper.ts (2)
16-20: Consider usingawaitfor consistency.The function is declared
asyncbut doesn't useawait. While this works (returning a Promise from an async function is valid), it's more idiomatic to useawait:export const getFile = async (path: string, encoding: BufferEncoding): Promise<string | Buffer> => { const readFile = promisify(fs.readFile); - return encoding ? readFile(path, { encoding }) : readFile(path, {}); + return encoding ? await readFile(path, { encoding }) : await readFile(path, {}); };
22-26: Consider usingawaitfor consistency.Similar to
getFile, this function is declaredasyncbut doesn't useawait. For consistency with async/await patterns:export const deleteFile = async (path: string): Promise<void> => { const unlink = promisify(fs.unlink); - return unlink(path); + await unlink(path); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api-gateway/src/helper-files/file-operation.helper.ts(1 hunks)apps/api-gateway/src/verification/interfaces/verification.interface.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api-gateway/src/verification/interfaces/verification.interface.ts (3)
apps/verification/src/interfaces/verification.interface.ts (2)
IProofRequestAttribute(4-12)IProofPresentation(219-222)apps/api-gateway/src/connection/interfaces/index.ts (1)
IUserRequestInterface(3-15)apps/agent-service/src/interface/agent-service.interface.ts (3)
IUserRequestInterface(237-250)IProofPresentation(528-537)IPresentation(481-485)
🔇 Additional comments (5)
apps/api-gateway/src/helper-files/file-operation.helper.ts (1)
14-14: LGTM!Clean utility function that properly wraps
fs.existsSync.apps/api-gateway/src/verification/interfaces/verification.interface.ts (4)
1-1: LGTM: Import formatting standardized.The import statement formatting is correctly standardized with single quotes.
11-18: LGTM: Interface formatting standardized.The formatting changes for IProofRequestSearchCriteria are appropriate ESLint fixes.
47-56: LGTM: Formatting standardized for proof format interfaces.The semicolon additions and spacing adjustments are appropriate ESLint fixes with no functional impact.
58-60: LGTM: Formatting standardized for remaining interfaces.The formatting changes for IRequestedAttributes, IRequestedPredicates, IRequestedPredicatesName, and IRequestedRestriction are appropriate ESLint fixes.
Also applies to: 68-84
| export interface IProofRequestAttribute { | ||
| attributeName: string; | ||
| condition?: string; | ||
| value?: string; | ||
| credDefId: string; | ||
| credentialName: string; | ||
| attributeName: string; | ||
| condition?: string; | ||
| value?: string; | ||
| credDefId: string; | ||
| credentialName: string; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Breaking changes detected: Required fields added to IProofRequestAttribute.
This interface has breaking changes that go beyond ESLint formatting:
- Line 4:
attributeNamechanged from optional to required - Line 7:
credDefIdchanged from optional to required - Line 8:
credentialNamechanged from optional to required
The relevant code snippet from apps/verification/src/interfaces/verification.interface.ts shows these were originally optional fields. Making them required will break any code that creates objects of this type without providing these fields.
Verify the impact of these changes across the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Find all usages of IProofRequestAttribute to assess breaking change impact
# Search for object literals or instantiations that may not provide the now-required fields
rg -n -C3 -P 'IProofRequestAttribute|attributeName|credDefId.*credentialName' \
-g '!*.interface.ts' -g '*.ts' -g '*.js'Length of output: 54000
🏁 Script executed:
#!/bin/bash
# Description: Read the exact interface definition to verify all fields
cat apps/api-gateway/src/verification/interfaces/verification.interface.ts | head -20Length of output: 102
🏁 Script executed:
#!/bin/bash
# Description: Locate the interface file and read its contents
fd -type f -name "verification.interface.ts" | head -5 | while read file; do
echo "=== File: $file ==="
cat "$file"
echo ""
doneLength of output: 170
🏁 Script executed:
#!/bin/bash
# Description: Find and read verification.interface.ts files with correct syntax
fd -name "verification.interface.ts" | while read file; do
echo "=== File: $file ==="
cat "$file"
echo ""
doneLength of output: 230
🏁 Script executed:
#!/bin/bash
# Description: Use rg to read complete interface definitions with context
echo "=== API Gateway Interface ==="
rg -A 10 "export interface IProofRequestAttribute" "apps/api-gateway/src/verification/interfaces/"
echo -e "\n=== Verification Module Interface ==="
rg -A 10 "export interface IProofRequestAttribute" "apps/verification/src/interfaces/"
echo -e "\n=== Search for credentialName field ==="
rg "credentialName" --type ts -B 2 -A 2Length of output: 3497
🏁 Script executed:
#!/bin/bash
# Description: Find usages of api-gateway IProofRequestAttribute interface
echo "=== Imports of api-gateway IProofRequestAttribute ==="
rg "from.*api-gateway.*verification.*IProofRequestAttribute|import.*IProofRequestAttribute" --type ts -B 1 -A 1
echo -e "\n=== Object literals or assignments to IProofRequestAttribute ==="
rg "IProofRequestAttribute\s*=|:\s*IProofRequestAttribute" --type ts -B 2 -A 5Length of output: 2260
🏁 Script executed:
#!/bin/bash
# Description: Check if api-gateway verification interface is exported and imported elsewhere
echo "=== Check api-gateway interface file for re-exports ==="
rg "export.*from.*verification.interface|export.*IProofRequestAttribute" "apps/api-gateway/src/verification/interfaces/verification.interface.ts"
echo -e "\n=== Check for imports from api-gateway verification interface ==="
rg "from.*api-gateway.*verification.*interfaces|from.*@credebl.*verification.*IProofRequestAttribute" --type ts -g '!apps/api-gateway' | head -20
echo -e "\n=== Check what api-gateway exports at index level ==="
fd -name "index.ts" | xargs grep -l "verification" | head -5 | xargs catLength of output: 450
🏁 Script executed:
#!/bin/bash
# Description: Find DTOs in api-gateway that use IProofRequestAttribute
echo "=== DTOs using IProofRequestAttribute in api-gateway ==="
rg "IProofRequestAttribute" "apps/api-gateway/src" --type ts -B 3 -A 3 | grep -E "(class|interface|export|@)" -A 3
echo -e "\n=== Check request-proof.dto implementation ==="
rg -A 20 "class ProofRequestAttribute" "apps/api-gateway/src/verification/dto/"Length of output: 4397
🏁 Script executed:
#!/bin/bash
# Description: Check how IProofRequestAttribute is used in validation/controllers
echo "=== Check where IProofRequestAttribute interface is used ==="
rg "IProofRequestAttribute" "apps/api-gateway/src" --type ts -B 2 -A 2 | head -60
echo -e "\n=== Check if verification controller uses the interface directly ==="
fd -name "*controller.ts" -path "*/api-gateway/*verification*" | xargs grep -l "IProofRequestAttribute\|ProofRequestAttribute"
echo -e "\n=== Check complete ProofRequestAttribute DTO for credentialName ==="
rg -A 50 "^export class ProofRequestAttribute" "apps/api-gateway/src/verification/dto/request-proof.dto.ts" | grep -E "(credentialName|credDefId|@)"Length of output: 2836
🏁 Script executed:
#!/bin/bash
# Description: Get complete ProofRequestAttribute DTO class definition
rg "export class ProofRequestAttribute" "apps/api-gateway/src/verification/dto/request-proof.dto.ts" -A 100 | head -120Length of output: 3252
Breaking changes to IProofRequestAttribute require careful review of downstream consumers.
The interface changes are confirmed as breaking:
attributeName: changed from optional to requiredcredDefId: changed from optional to requiredcredentialName: changed from optional to required
These fields were optional in both the original verification module interface and the common library interface. Additionally, there's a critical mismatch: the ProofRequestAttribute DTO does not include a credentialName field at all, yet the interface requires it. This inconsistency could cause runtime issues if code tries to use the interface and instantiate objects without providing this field.
Any code that creates or processes IProofRequestAttribute objects without providing all three required fields will break. Verify usage patterns across the codebase and update calling code accordingly.
🤖 Prompt for AI Agents
In apps/api-gateway/src/verification/interfaces/verification.interface.ts around
lines 3 to 9, the interface IProofRequestAttribute was changed to make
attributeName, credDefId and credentialName required but the
ProofRequestAttribute DTO and downstream callers expect these to be optional;
also credentialName is not present in the DTO, causing a mismatch and potential
runtime breaks. Fix by restoring the original optionality for attributeName and
credDefId (and credentialName) OR add credentialName to the DTO and update all
callers to supply it: run a repo-wide search for IProofRequestAttribute and
ProofRequestAttribute usages, update affected creators/parsers to provide the
new fields if you choose to require them, or revert the interface to optional
fields to maintain backward compatibility, then run typechecks and
unit/integration tests to validate no consumers break.
| export interface IPresentation extends IProofPresentation { | ||
| _tags: ITags; | ||
| metadata: object; | ||
| id: string; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the structural change to IPresentation extending IProofPresentation.
While the inheritance structure appears correct based on the agent-service interfaces, this is a significant structural change beyond ESLint formatting. IPresentation now inherits all fields from IProofPresentation (createdAt, protocolVersion, state, connectionId, threadId, autoAcceptProof, updatedAt, isVerified) in addition to its own fields (_tags, metadata, id).
Verify that all usages of IPresentation are compatible with the extended interface:
🏁 Script executed:
#!/bin/bash
# Description: Find all usages of IPresentation to verify compatibility with the new structure
# Search for IPresentation usage patterns
rg -n -C5 -P ':\s*IPresentation|<IPresentation>|IPresentation\s*\{' \
-g '*.ts' -g '*.js'Length of output: 4666
🏁 Script executed:
#!/bin/bash
# Find invocations of verifyPresentation method
rg -n 'verifyPresentation\s*\(' -g '*.ts' -g '*.js' -A2 -B2Length of output: 4555
🏁 Script executed:
#!/bin/bash
# Find imports of IPresentation to understand usage patterns
rg -n 'import.*IPresentation|from.*verification' -g '*.ts' -g '*.js' -C2Length of output: 12301
🏁 Script executed:
#!/bin/bash
# Find the _verifyPresentation method implementation to see what it actually returns
rg -n '_verifyPresentation\(' -A20 'apps/verification/src/verification.service.ts'Length of output: 1649
🏁 Script executed:
#!/bin/bash
# Check the full context of verification.service verifyPresentation to understand response handling
rg -n 'async verifyPresentation' -A15 'apps/verification/src/verification.service.ts'Length of output: 797
🏁 Script executed:
#!/bin/bash
# Verify how agent-service returns verification data
rg -n 'async verifyPresentation' -A10 'apps/agent-service/src/agent-service.service.ts'Length of output: 674
🏁 Script executed:
#!/bin/bash
# Check what IPresentation actually looks like in api-gateway before the change
# Search for references to verify if IPresentation was previously something different
rg -n 'interface IPresentation|type IPresentation' -B2 -A8 'apps/api-gateway/src/verification/interfaces/verification.interface.ts'Length of output: 207
Critical type mismatch in verifyPresentation() method—cannot fulfill declared IPresentation return type.
The structural change to IPresentation extending IProofPresentation is valid, but there's a critical type system violation: api-gateway's verifyPresentation() method declares it returns Promise<IPresentation> (line 86, verification.service.ts), but the verification service actually returns Promise<string> (line 402 in apps/verification/src/verification.service.ts). The verification service extracts only the .response field (a string) from the agent response and returns it via NATS.
This breaks the type contract in api-gateway's controller (line 379), which expects to work with an IPresentation object. The IPresentation interface extension is sound, but the runtime data being returned does not match the promised type.
Verify: (1) whether this is a pre-existing issue or introduced by this PR, and (2) whether the NATS client provides type deserialization that reconciles this mismatch. If NATS doesn't handle deserialization, api-gateway's return type should be changed to Promise<string> or the verification service should return the full object.
🤖 Prompt for AI Agents
In apps/api-gateway/src/verification/interfaces/verification.interface.ts lines
35-39, IPresentation was changed to extend IProofPresentation but the
api-gateway verifyPresentation() return type (Promise<IPresentation>) doesn't
match the actual data sent over NATS (a string response). Inspect
apps/verification/src/verification.service.ts (around line 402) and the NATS
call in api-gateway (verification.service.ts line ~86 and controller line ~379)
to determine whether the NATS client automatically deserializes the full object;
if NATS does not deserialize the full IPresentation, either (A) change the
api-gateway verifyPresentation() return type to Promise<string> and update its
controller/consumers accordingly, or (B) modify the verification service to
return the complete IPresentation object (not just .response) and ensure NATS
publishes that full object with proper serialization; implement the chosen fix
consistently across both services and update types/tests.
| interface IRequestedAttributesName { | ||
| name?: string; | ||
| names?: string; | ||
| restrictions: IRequestedRestriction[] | ||
| name?: string; | ||
| names?: string; | ||
| restrictions: IRequestedRestriction[]; | ||
| } |
There was a problem hiding this comment.
Critical: Type error in IRequestedAttributesName.names property.
Line 64 has an incorrect type change: names?: string; should be names?: string[];
The property name is plural ("names"), indicating it should be an array. This change from string[] to string is not an ESLint formatting fix—it's a breaking type change that will cause runtime errors when code attempts to iterate over or access this property as an array.
Apply this diff to restore the correct type:
interface IRequestedAttributesName {
name?: string;
- names?: string;
+ names?: string[];
restrictions: IRequestedRestriction[];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface IRequestedAttributesName { | |
| name?: string; | |
| names?: string; | |
| restrictions: IRequestedRestriction[] | |
| name?: string; | |
| names?: string; | |
| restrictions: IRequestedRestriction[]; | |
| } | |
| interface IRequestedAttributesName { | |
| name?: string; | |
| names?: string[]; | |
| restrictions: IRequestedRestriction[]; | |
| } |
🤖 Prompt for AI Agents
In apps/api-gateway/src/verification/interfaces/verification.interface.ts around
lines 62 to 66, the IRequestedAttributesName.names property is incorrectly typed
as string; change it back to an array type names?: string[]; so the plural
property matches its intended usage and callers that iterate or index it won’t
break. Ensure the file compiles by updating the type annotation only (no other
behavior changes) and run type checks to confirm the fix.
1 participant
fixed eslint issues
Summary by CodeRabbit