Skip to content
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

bugfix/aura-logging #692

Closed
wants to merge 25 commits into from
Closed

bugfix/aura-logging #692

wants to merge 25 commits into from

Conversation

ZackFra
Copy link

@ZackFra ZackFra commented May 26, 2024

Found that trying to log via aura components didn't format correctly. Raised an issue about it.

I expect this to get rejected, since I am aware that some of the Logger_Test methods are failing. Didn't want to mess with them without some sort of input. Looks like they might be using old stack traces that Salesforce doesn't generate anymore, that's my best guess anyway.

In a nutshell, found that aura components can't pull in the createLogger function that's exported by the logger lwc, and the stack trace captured by info, debug, etc. won't capture the aura component in the stack trace when you call them from component (i.e. component.find('logger').info('...').

Updated the logger LWC to have a createLogger function that exports this. After calling this new createLogger function, it works correctly.

There were issues with generating the stack trace in JS when used in a callback (i.e. createLogger().then( () => { ... }) so refacted createLogger to be synchronous, and to use JIT logic to pull in the settings. i.e. if the settings aren't loaded, saveLog will just await the this.#settingsPromise until it resolves.

Updated the LoggerStackTrace apex class to parse the stack traces from JavaScript with updated logic.

@ZackFra ZackFra requested a review from jongpie as a code owner May 26, 2024 00:57
@jongpie
Copy link
Owner

jongpie commented Jun 12, 2024

@ZackFra thanks for reporting this & submitting a PR! And apologies for the late response, I've had limited time lately to work on open source stuff, but hope to give this a more thorough review in the coming weeks.

Offhand, I have a couple of concerns with this PR - it looks like it changes/breaks logging in existing aura components & LWCs

  • Aura components will now need to call createLogger(), which breaks existing implementations
  • LWCs would potentially have issues with createLogger() no longer being async
  • The change to _meetsUserLoggingLevel(logEntryLoggingLevel) (snippet below) will cause a bug in logging - by returning true when settings haven't been loaded, it will end up logging entries that shouldn't be logged. The same approach was used in Nebula Logger a while ago (last year?), and was changed to avoid this problem
  _meetsUserLoggingLevel(logEntryLoggingLevel) {
    // return true until settings are loaded
    if(!this.#isSettingsLoaded) {
      return true;
    }
    ...
  }

I'll review + test this more ASAP!

@jongpie
Copy link
Owner

jongpie commented Aug 6, 2024

I'm closing this PR, and will instead merge #727, which moves JS stack trace parsing to happen in JS directly (instead of in Apex). Some of the changes in this PR have been incorporated into #727, but since the entire architecture was changed, it was cleaner to create a new branch/PR for it.

Some related info for extra context & future reference:

@jongpie jongpie closed this Aug 6, 2024
@jongpie jongpie added Type: Bug Something isn't working Logging Source: Lightning Components Items related to using Nebula Logger using JavaScript within lightning components (lwc & aura) Layer: Logger Engine Items related to the core logging engine labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layer: Logger Engine Items related to the core logging engine Logging Source: Lightning Components Items related to using Nebula Logger using JavaScript within lightning components (lwc & aura) Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants