Skip to content

Commit

Permalink
Added protection against log modules logging against themselves when …
Browse files Browse the repository at this point in the history
…using createLogger

A log module is a log sink. It is passed a createLogger function as it is a module but if used it would result in the logged messages being passed back to itself (as well as other log sinks).

An additional check has been done when initializing log modules to prevent the log sink linked logger being passed. A local logger is returned that logs just to the console (for debugging purposes) when called from a log module.
  • Loading branch information
johnman committed Oct 17, 2024
1 parent 5b7d5e6 commit 9876f38
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 2 deletions.
1 change: 1 addition & 0 deletions how-to/workspace-platform-starter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
- Updated example-notification-source so that you can specify a url to post notifications to (it will wrap the request in a message property and send it to the server you specify via post: { url: "" } settings. Our example node-starter supports messages being posted to <http://localhost:6060/api/messages>.
- Updated the notification service example app so that if you select the endpoint example action it will post a notification to the notification source create endpoint.
- BUGFIX - Updated Snapshot launching logic as there was a bug when updating a page's layout (to ensure view names are regenerated when using app naming conventions) in a multi page window which resulted in the page layout being assigned at the wrong level.
- Enhancement - Previously if you were generating a log module you needed to avoid using the injected createLogger function as you would end up calling yourself and ending up in a loop. The module logic has been updated to pass an alternative createLogger function to log modules that only logs to the console (so that it can still be used for debugging) but doesn't call itself or other log sinks (log modules).

## v19.0.0

Expand Down
37 changes: 35 additions & 2 deletions how-to/workspace-platform-starter/client/src/framework/modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export async function loadModules<
const modules = moduleList?.modules;
if (Array.isArray(modules)) {
for (const moduleDefinition of modules) {
moduleDefinition.moduleType = moduleType;
const module = await loadModule<M, H, O, D>(moduleDefinition, moduleType);
if (module) {
loaded.push(module);
Expand Down Expand Up @@ -210,13 +211,22 @@ export async function initializeModule<
if (!moduleEntry.isInitialized) {
if (moduleEntry.implementation?.initialize) {
try {
logger.info(`Initializing module '${moduleEntry.definition.id}'`);
logger.info(
`Initializing module '${moduleEntry.definition.id}' of type '${moduleEntry.definition.moduleType}'`
);
const moduleCreateLogger =
moduleEntry.definition.moduleType === "log" ? createLocalLogger : createLogger;

const moduleHelpers = {
...helpers,
getNotificationClient: getNotificationClientProxy(moduleEntry.definition),
getEndpointClient: getEndpointClientProxy(moduleEntry.definition)
};
await moduleEntry.implementation.initialize(moduleEntry.definition, createLogger, moduleHelpers);
await moduleEntry.implementation.initialize(
moduleEntry.definition,
moduleCreateLogger,
moduleHelpers
);
moduleEntry.isInitialized = true;
} catch (err) {
logger.error(`Error initializing module ${moduleEntry.definition.id}`, err);
Expand Down Expand Up @@ -456,3 +466,26 @@ async function getDialogClient(): Promise<DialogClient> {
showConfirmation: Dialog.showConfirmation
};
}

/**
* Create local logger.
* @param group The group to encapsulate the loge entries with.
* @returns The local logger that uses the console and doesn't send messages to the log modules.
*/
export function createLocalLogger(group: string): Logger {
console.info(
`${group}: Creating local logger for a log module. Log modules will not be able to use the logger to send messages to log modules as it would be sending messages to itself as well as other log sinks. This will be logged to the console of the provider.`
);
return {
info: (message: unknown, ...optionalParams: unknown[]) =>
console.log(`${group}: ${message}`, ...optionalParams),
warn: (message: unknown, ...optionalParams: unknown[]) =>
console.warn(`${group}: ${message}`, ...optionalParams),
error: (message: unknown, ...optionalParams: unknown[]) =>
console.error(`${group}: ${message}`, ...optionalParams),
trace: (message: unknown, ...optionalParams: unknown[]) =>
console.debug(`${group}: ${message}`, ...optionalParams),
debug: (message: unknown, ...optionalParams: unknown[]) =>
console.debug(`${group}: ${message}`, ...optionalParams)
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ export interface ModuleDefinition<O = unknown> {
* Custom data for the module.
*/
data?: O;

/**
* This is specified by the provider while loading the modules.
*/
moduleType?: ModuleTypes;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ export interface ModuleDefinition<O = unknown> {
* Custom data for the module.
*/
data?: O;
/**
* This is specified by the provider while loading the modules.
*/
moduleType?: ModuleTypes;
}
/**
* Helper methods and data to pass to the modules during initialization.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2075,6 +2075,10 @@
"description": "Url to more information.",
"type": "string"
},
"moduleType": {
"$ref": "#/definitions/ModuleTypes",
"description": "This is specified by the provider while loading the modules."
},
"moduleUrl": {
"description": "This is the old property, it will be remapped to url.",
"type": "string"
Expand Down Expand Up @@ -2313,6 +2317,10 @@
"description": "Url to more information.",
"type": "string"
},
"moduleType": {
"$ref": "#/definitions/ModuleTypes",
"description": "This is specified by the provider while loading the modules."
},
"title": {
"description": "The title of the module.",
"type": "string"
Expand Down Expand Up @@ -3081,6 +3089,10 @@
"description": "Url to more information.",
"type": "string"
},
"moduleType": {
"$ref": "#/definitions/ModuleTypes",
"description": "This is specified by the provider while loading the modules."
},
"title": {
"description": "The title of the module.",
"type": "string"
Expand Down Expand Up @@ -3210,6 +3222,26 @@
},
"type": "object"
},
"ModuleTypes": {
"description": "The possible module types.",
"enum": [
"actions",
"analytics",
"auth",
"conditions",
"contentCreation",
"endpoint",
"initOptions",
"integrations",
"interopOverride",
"lifecycle",
"log",
"menus",
"platformOverride",
"share"
],
"type": "string"
},
"MonitorDetails": {
"$ref": "#/definitions/__type_49"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1948,6 +1948,10 @@
"description": "Url to more information.",
"type": "string"
},
"moduleType": {
"$ref": "#/definitions/ModuleTypes",
"description": "This is specified by the provider while loading the modules."
},
"moduleUrl": {
"description": "This is the old property, it will be remapped to url.",
"type": "string"
Expand Down Expand Up @@ -2180,6 +2184,10 @@
"description": "Url to more information.",
"type": "string"
},
"moduleType": {
"$ref": "#/definitions/ModuleTypes",
"description": "This is specified by the provider while loading the modules."
},
"title": {
"description": "The title of the module.",
"type": "string"
Expand Down Expand Up @@ -2879,6 +2887,10 @@
"description": "Url to more information.",
"type": "string"
},
"moduleType": {
"$ref": "#/definitions/ModuleTypes",
"description": "This is specified by the provider while loading the modules."
},
"title": {
"description": "The title of the module.",
"type": "string"
Expand Down Expand Up @@ -3000,6 +3012,26 @@
},
"type": "object"
},
"ModuleTypes": {
"description": "The possible module types.",
"enum": [
"actions",
"analytics",
"auth",
"conditions",
"contentCreation",
"endpoint",
"initOptions",
"integrations",
"interopOverride",
"lifecycle",
"log",
"menus",
"platformOverride",
"share"
],
"type": "string"
},
"MonitorDetails": {
"$ref": "#/definitions/__type_49"
},
Expand Down

0 comments on commit 9876f38

Please sign in to comment.