-
Notifications
You must be signed in to change notification settings - Fork 16
improved error messages #2175
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
base: development
Are you sure you want to change the base?
improved error messages #2175
Conversation
reeshika-h
commented
Oct 15, 2025
- Phase 1 : covered around 500 lines
…ity across various modules
| await publishOnlyUnpublishedService.apply(this, [PublishOnlyUnpublished]); | ||
| } catch (error) { | ||
| this.error(error, { exit: 2 }); | ||
| this.error(error.message || error, { exit: 2 }); |
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.
@reeshika-h Please use option chaining.
this.error(error?.message || error, { exit: 2 });
| config.source_stack = listOfTokens[sourceManagementTokenAlias].apiKey; | ||
| } else if (sourceManagementTokenAlias) { | ||
| console.log(`Provided source token alias (${sourceManagementTokenAlias}) not found in your config.!`); | ||
| console.log(`The source token alias '${sourceManagementTokenAlias}' was not found in your configuration file.`); |
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.
@reeshika-h Please verify, its not a configuration file
| const result = spawnSync('csdx', ['config:set:region', 'AZURE-NA'], { encoding: 'utf-8' }); | ||
| const output = result.stdout + result.stderr; | ||
|
|
||
| console.log(output); |
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.
@reeshika-h Remove console.log
| const result = spawnSync('csdx', ['config:set:region', 'AWS-NA'], { encoding: 'utf-8' }); | ||
| const output = result.stdout + result.stderr; | ||
|
|
||
| console.log(output); |
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.
@reeshika-h Remove console.log
| }; | ||
| } else if (managementTokenAlias) { | ||
| this.error('Provided management token alias not found in your config.!'); | ||
| this.error('The provided management token alias was not found in your configuration file.'); |
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.
@reeshika-h Please verify, Its not a configuration file.
|
|
||
| log.debug(`Assets root path resolved to: ${this.assetsRootPath}`, this.exportConfig.context); | ||
| log.debug('Fetching assets and folders count...', this.exportConfig.context); | ||
| log.debug('Fetching asset and folder count...', this.exportConfig.context); |
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.
@reeshika-h Use plural here because this process fetches counts in bulk.
| async start() { | ||
| try { | ||
| log.debug('Starting content types export process...', this.exportConfig.context); | ||
| log.debug('Starting content type export process...', this.exportConfig.context); |
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.
@reeshika-h Use plural here because this process in bulk.
| async start() { | ||
| try { | ||
| log.debug('Starting entries export process...', this.exportConfig.context); | ||
| log.debug('Starting entry export process...', this.exportConfig.context); |
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.
@reeshika-h Use plural here because this process exports in bulk.
|
|
||
| await this.getLocales(); | ||
| log.debug(`Retrieved ${Object.keys(this.locales).length} locales and ${Object.keys(this.masterLocale).length} master locales`, this.exportConfig.context); | ||
| log.debug(`Retrieved ${Object.keys(this.locales).length} locales.`, this.exportConfig.context); |
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.
@reeshika-h Please revert to the original log message (locales + master locales).
| }); | ||
|
|
||
| log.debug(`Sanitization complete. Master locales: ${Object.keys(this.masterLocale).length}, Regular locales: ${Object.keys(this.locales).length}`, this.exportConfig.context); | ||
| log.debug(`Sanitization complete. Master locales: ${Object.keys(this.masterLocale).length}.`, this.exportConfig.context); |
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.
@reeshika-h Please revert to the original log message (locales + master locales).
|
|
||
| if (this.exportConfig.personalizationEnabled) { | ||
| log.debug('Personalization is enabled, processing personalize modules...', this.exportConfig.context); | ||
| log.debug('Personalization is enabled.', this.exportConfig.context); |
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.
@reeshika-h Please revert to the original log message.
|
|
||
| if (this.exportConfig.management_token) { | ||
| log.debug('Management token detected, skipping personalize export', this.exportConfig.context); | ||
| log.debug('Management token detected.', this.exportConfig.context); |
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.
@reeshika-h Please revert to the original log message.
| log.debug('Completed all personalization module exports.', this.exportConfig.context); | ||
| } else { | ||
| log.debug('Personalization is disabled, skipping personalize module exports', this.exportConfig.context); | ||
| log.debug('Personalization is disabled.', this.exportConfig.context); |
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.
@reeshika-h Please revert to the original log message.
| this.customRolesLocales = fsUtil.readFile(join(this.customRolesFolderPath, this.customRolesConfig.customRolesLocalesFileName),true) as Record<string, unknown>; | ||
| } else { | ||
| log.info(`No custom-rules are found - '${this.customRolesFolderPath}'`, this.importConfig.context); | ||
| log.info(`No custom rules found in: ${this.customRolesFolderPath}`, this.importConfig.context); |
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.
@reeshika-h Its a custom roles not rules.
| this.locales = values((fsUtil.readFile(this.localesPath) || []) as Record<string, unknown>[]); | ||
| this.locales.unshift(this.importConfig.master_locale); // adds master locale to the list | ||
| log.debug(`Processing entries for ${values(this.locales).length} locales`, this.importConfig.context); | ||
| log.debug(`Processing entries for ${values(this.locales)}`, this.importConfig.context); |
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.
@reeshika-h Please revert to the original log message.
| log.debug(`Environment '${name}' (${uid}) failed to import`, this.importConfig.context); | ||
| if (err?.errors?.name) { | ||
| log.debug(`Environment '${name}' already exists, fetching details`, this.importConfig.context); | ||
| log.debug(`Environment '${name}' already exists`, this.importConfig.context); |
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.
@reeshika-h Please revert to the original log message.
| return; | ||
| } | ||
| log.debug(`Found ${values(this.languages).length} languages to import`, this.config.context); | ||
| log.debug(`Found ${Object.values(this.languages).length} language entries`, this.config.context); |
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.
@reeshika-h Please revert to the original log message.
| const onReject = ({ error, apiData: { uid, code } = undefined }: any) => { | ||
| if (error?.errorCode === 247) { | ||
| log.info(formatError(error), this.config.context); | ||
| log.info(`Format error: ${error}`, this.config.context); |
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.
@reeshika-h Use log.error here and ensure we call formatError(error) instead of logging the raw error.
| // NOTE update configurations | ||
| if (updateParam && (!isEmpty(updateParam.configuration) || !isEmpty(updateParam.server_configuration))) { | ||
| log.debug(`Updating app configuration for: ${app.manifest?.name}`, this.importConfig.context); | ||
| log.debug(`Updating configuration for: ${app.manifest?.name}`, this.importConfig.context); |
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.
@reeshika-h Please revert to the original log message.
| if (error?.status === 409 && error?.statusText === 'Conflict') { | ||
| log.info( | ||
| `Taxonomy '${taxonomyUID}' already exists${locale ? ` for locale: ${locale}` : ''}!`, | ||
| `Taxonomy '${taxonomyUID}' already exists.`, |
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.
@reeshika-h Please revert to the original log message
| if (errMsg) { | ||
| log.error( | ||
| `Taxonomy '${taxonomyUID}' failed to import${locale ? ` for locale: ${locale}` : ''}! ${errMsg}`, | ||
| `Failed to import taxonomy '${taxonomyUID}': ${errMsg}`, |
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.
@reeshika-h Please revert to the original log message
| // Else warn and modify the schema object. | ||
| isContentTypeError = true; | ||
| log.warn(`Content-type ${schema[i].reference_to[j]} does not exist. Removing the field from schema`); | ||
| console.log(`Content type ${schema[i].reference_to[j]} does not exist. Removing the field from schema...`); |
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.
@reeshika-h Please revert to the original log message
|
|
||
| fsUtil.writeFile(this.attributesUidMapperPath, this.attributesUidMapper); | ||
| log.debug(`Saved ${Object.keys(this.attributesUidMapper).length} attribute mappings to: ${this.attributesUidMapperPath}`, this.config.context); | ||
| log.debug(`Saved ${Object.keys(this.attributesUidMapper).length} attribute mappings`, this.config.context); |
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.
@reeshika-h Please revert to the original log message
| log.debug(`Processing ${definition.rules.length} definition rules for audience: ${name}`, this.config.context); | ||
| definition.rules = lookUpAttributes(definition.rules, attributesUid); | ||
| log.debug(`Processed definition rules, remaining rules: ${definition.rules.length}`, this.config.context); | ||
| log.debug(`Processed definition rules.`, this.config.context); |
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.
@reeshika-h Please revert to the original log message
|
|
||
| fsUtil.writeFile(this.audiencesUidMapperPath, this.audiencesUidMapper); | ||
| log.debug(`Saved ${Object.keys(this.audiencesUidMapper).length} audience mappings to: ${this.audiencesUidMapperPath}`, this.config.context); | ||
| log.debug(`Saved ${Object.keys(this.audiencesUidMapper).length} audience mappings`, this.config.context); |
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.
@reeshika-h Please revert to the original log message
|
|
||
| fsUtil.writeFile(this.eventsUidMapperPath, this.eventsUidMapper); | ||
| log.debug(`Saved ${Object.keys(this.eventsUidMapper).length} event mappings to: ${this.eventsUidMapperPath}`, this.config.context); | ||
| log.debug(`Saved ${Object.keys(this.eventsUidMapper).length} event mappings`, this.config.context); |
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.
@reeshika-h Please revert to the original log message
| error.includes('personalize.PROJECTS.DUPLICATE_NAME') | ||
| ) { | ||
| log.warn(`Project name already exists, generating new name`, this.config.context); | ||
| log.warn(`Project name already exists.`, this.config.context); |
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.
@reeshika-h Please revert to the original log message
| ): CreateExperienceInput => { | ||
| log.debug('Starting audience lookup for experience'); | ||
| log.debug(`Available audience mappings: ${Object.keys(audiencesUid)?.length}`); | ||
| log.debug(`Available audience mappings: ${Object.keys(audiencesUid).length}`); |
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.
@reeshika-h Use optional chaining as earlier
| ): CreateExperienceInput => { | ||
| log.debug('Starting event lookup for experience'); | ||
| log.debug(`Available event mappings: ${Object.keys(eventsUid)?.length}`); | ||
| log.debug(`Available event mappings: ${Object.keys(eventsUid).length}`); |
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.
@reeshika-h Use optional chaining as earlier