Skip to content

Commit

Permalink
fix(@sap-ux/odata-service-inquirer): No log method when unparseae…
Browse files Browse the repository at this point in the history
…ble metadata results in unhandled exception (#2895)

* fix entity-helper.ts

There is no 'log' method on the passed ILogWrapper interface yet and so calling this will cause a runtime exception. Changing for now to error as it should be. Will follow-up TBI to add `log` to `ILogWrapper` as passed from YUI.

* Create witty-flies-drive.md

* Update witty-flies-drive.md

* Update questions.test.ts

Update test

* Adds logger compatibility between `@vscode-logging/logger` and
`@sap-ux/logger`

#2896

* Updates cset

* Adds tests to cover
  • Loading branch information
IainSAP authored Feb 13, 2025
1 parent f1fbb34 commit fb4e328
Show file tree
Hide file tree
Showing 11 changed files with 247 additions and 103 deletions.
7 changes: 7 additions & 0 deletions .changeset/flat-cougars-sleep.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@sap-ux/fiori-generator-shared': minor
"@sap-ux/odata-service-inquirer": patch
---

Adds interoperability between `@vscode-logging/logger` and `@sap-ux/logger` to prevent crashes where non-implemented log functions were being called.
Fix entity-helper.ts to log error at correct level.
3 changes: 2 additions & 1 deletion packages/fiori-generator-shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@
"@types/mem-fs": "1.1.2",
"@types/semver": "7.5.2",
"@types/vscode": "1.73.1",
"@types/yeoman-environment": "2.10.11"
"@types/yeoman-environment": "2.10.11",
"@sap-ux/logger": "workspace:*"
},
"engines": {
"node": ">=18.x"
Expand Down
105 changes: 93 additions & 12 deletions packages/fiori-generator-shared/src/logging/logWrapper.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { Log, Logger as SapUxLogger, Transport } from '@sap-ux/logger';
import type {
getExtensionLoggerOpts,
IChildLogger as ILogWrapper,
IVSCodeExtLogger,
LogLevel,
IChildLogger as ILogWrapper
LogLevel
} from '@vscode-logging/logger';
import { getExtensionLogger } from '@vscode-logging/logger';
import { format } from 'logform';
Expand Down Expand Up @@ -38,7 +39,31 @@ export const DefaultLogger: LogWrapper = {
console.trace(msg);
},
getChildLogger: () => DefaultLogger,
getLogLevel: () => 'off'
getLogLevel: () => 'off',
/**
* Limited compatibility with `@sap-ux/logger`
*
* @param data
*/
log: function (data: string | Log): void {
console.log(data instanceof Object ? data.message : data);
},
add: function (): SapUxLogger {
console.warn('Log method `add(transport)` not implemented.');
return this;
},
remove: function (): SapUxLogger {
console.warn('Log method `remove(transport)` not implemented.');
return this;
},
transports: function (): Transport[] {
console.warn('Logger method `transports()` not implemented.');
return [];
},
child: function (): SapUxLogger {
console.warn('Log method `remove(transport)` not implemented. Returning current logger.');
return this;
}
};

const LOG_LEVEL_KEYS: Record<LogLevel, number> = {
Expand Down Expand Up @@ -71,9 +96,9 @@ export function createCLILogger(logName: string, logLevel: LogLevel = 'off'): IL
/**
* Log to vscode extension logger and yeoman logger simultaneously.
* This allows use of Application Wizard log config and log file use but still have a single output channel for
* App Gen logging.
* generator logging.
*/
export class LogWrapper implements ILogWrapper {
export class LogWrapper implements ILogWrapper, SapUxLogger {
private static _vscodeLogger: ILogWrapper;
private static _yoLogger: Logger | undefined;
private static _logLevel: LogLevel;
Expand Down Expand Up @@ -110,7 +135,11 @@ export class LogWrapper implements ILogWrapper {
LogWrapper._vscodeLogger?.debug(t('debug.loggingConfigured', { logLevel: LogWrapper._logLevel }));
}

static readonly logAtLevel = (level: LogLevel, message: string, ...args: any[]) => {
static readonly logAtLevel = (level: LogLevel, message: string | object, ...args: any[]) => {
if (typeof message === 'object') {
message = JSON.stringify(message);
}

if (LogWrapper._vscodeLogger && level !== 'off') {
LogWrapper._vscodeLogger[level](message, ...args);
}
Expand Down Expand Up @@ -146,7 +175,7 @@ export class LogWrapper implements ILogWrapper {
* @param msg - message to log
* @param {...any} args - additional arguments
*/
error(msg: string, ...args: any[]): void {
error(msg: string | object, ...args: any[]): void {
LogWrapper.logAtLevel('error', msg, ...args);
}
/**
Expand All @@ -155,7 +184,7 @@ export class LogWrapper implements ILogWrapper {
* @param msg - message to log
* @param {...any} args - additional arguments
*/
warn(msg: string, ...args: any[]): void {
warn(msg: string | object, ...args: any[]): void {
LogWrapper.logAtLevel('warn', msg, ...args);
}
/**
Expand All @@ -164,7 +193,7 @@ export class LogWrapper implements ILogWrapper {
* @param msg - message to log
* @param {...any} args - additional arguments
*/
info(msg: string, ...args: any[]): void {
info(msg: string | object, ...args: any[]): void {
LogWrapper.logAtLevel('info', msg, ...args);
}
/**
Expand All @@ -173,7 +202,7 @@ export class LogWrapper implements ILogWrapper {
* @param msg - message to log
* @param {...any} args - additional arguments
*/
debug(msg: string, ...args: any[]): void {
debug(msg: string | object, ...args: any[]): void {
LogWrapper.logAtLevel('debug', msg, ...args);
}
/**
Expand All @@ -191,7 +220,7 @@ export class LogWrapper implements ILogWrapper {
*
* @param msg - message to log
*/
public static log(msg: string): void {
public static log(msg: string | object): void {
LogWrapper.logAtLevel('info', msg);
}

Expand All @@ -204,7 +233,59 @@ export class LogWrapper implements ILogWrapper {
return LogWrapper._logLevel;
}

/**
* Not implemented method, added to support limited interoperability with @sap-ux/logger.
*
* @returns {ILogWrapper} - the current logger
*/
getChildLogger(/* opts: { label: string } */): ILogWrapper {
throw new Error(t('error.methodNotImplemented'));
LogWrapper.logAtLevel(`trace`, 'Log method `getChildLogger()` not implemented. Returning current logger.');
return this;
}

/**
* Limited compatibility with `@sap-ux/logger` to use log() method.
*
* @param data
*/
log(data: string | Log): void {
// LogLevel is not supported in this implementation
LogWrapper.logAtLevel('info', (data as Log).message ?? data);
}
/**
* Not implemented method, added to support limited interoperability with @sap-ux/logger.
*
* @returns {ILogWrapper} - the current logger
*/
add(): SapUxLogger {
LogWrapper.logAtLevel(`warn`, 'Log method `add(transport)` not implemented.');
return this;
}
/**
* Not implemented method, added to support limited interoperability with @sap-ux/logger.
*
* @returns {ILogWrapper} - the current logger
*/
remove(): SapUxLogger {
LogWrapper.logAtLevel(`warn`, 'Log method `remove(transport)` not implemented.');
return this;
}
/**
* Added to support limited interoperability with @sap-ux/logger.
*
* @returns {ILogWrapper} - the current logger transports
*/
transports(): Transport[] {
LogWrapper.logAtLevel(`warn`, 'Log method `transports()` not implemented.');
return [];
}
/**
* Not implemented method, added to support limited interoperability with @sap-ux/logger.
*
* @returns {ILogWrapper} - the current logger
*/
child(): SapUxLogger {
LogWrapper.logAtLevel(`warn`, 'Log method `child(options)` not implemented. Returning current logger.');
return this;
}
}
88 changes: 0 additions & 88 deletions packages/fiori-generator-shared/test/logWrapper.test.ts

This file was deleted.

Loading

0 comments on commit fb4e328

Please sign in to comment.