-
Notifications
You must be signed in to change notification settings - Fork 16
refactor: CLI error handling and updates how authentication metadata is captured in import/export commands #2003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the CLI error handling and updates how authentication metadata is captured in import/export commands.
- Introduces new helper methods (
extractClearMessage
, streamlined normalization/classification, enhanced metadata extraction) inCLIErrorHandler
. - Standardizes
authMethod
values, makes it optional in config types, and updates context creation in import/export commands. - Renames context identifiers (
userId
→userUid
,organization_uid
→oauthOrgUid
) and adjustsBaseCommand
.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
packages/contentstack-utilities/src/logger/cli-error-handler.ts | Refactored error handler: added extractClearMessage , improved metadata and normalization logic |
packages/contentstack-import/src/utils/import-config-handler.ts | Standardized authMethod strings |
packages/contentstack-import/src/types/import-config.ts | Made authMethod optional |
packages/contentstack-import/src/commands/cm/stacks/import.ts | Updated createImportContext to include authMethod and renamed IDs |
packages/contentstack-export/src/utils/export-config-handler.ts | Standardized authMethod strings |
packages/contentstack-export/src/types/export-config.ts | Made authMethod optional |
packages/contentstack-export/src/commands/cm/stacks/export.ts | Updated createExportContext to include authMethod and renamed IDs |
packages/contentstack-auth/src/base-command.ts | Renamed context IDs |
Comments suppressed due to low confidence (3)
packages/contentstack-utilities/src/logger/cli-error-handler.ts:86
- This new helper method contains several conditional branches; consider adding unit tests for
extractClearMessage
to verify message formatting for API, network, and generic errors.
private extractClearMessage(error: Error & Record<string, any>): string {
packages/contentstack-utilities/src/logger/cli-error-handler.ts:83
- [nitpick] The JSDoc for
extractClearMessage
only has a summary. It would be helpful to add@param error
and@returns
tags to clarify expected input and output.
/**
packages/contentstack-utilities/src/logger/cli-error-handler.ts:222
- [nitpick] The new metadata extractor no longer includes
apiKey
from the context. If retainingapiKey
in error metadata is important for debugging, consider re-adding it or documenting its removal.
private extractMeta(context?: ErrorContext, errorType?: string): Record<string, string | undefined> {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR refactors the CLI error handling and updates how authentication metadata is captured in import/export commands.
Introduces new helper methods (extractClearMessage, streamlined normalization/classification, enhanced metadata extraction) in CLIErrorHandler.
Standardizes authMethod values, makes it optional in config types, and updates context creation in import/export commands.
Renames context identifiers (userId → userUid, organization_uid → oauthOrgUid) and adjusts BaseCommand.