Skip to content

Feature request: flush buffer on uncaught error - class method decorator #3635

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

Closed
1 of 2 tasks
dreamorosi opened this issue Feb 19, 2025 · 2 comments · Fixed by #3676
Closed
1 of 2 tasks

Feature request: flush buffer on uncaught error - class method decorator #3635

dreamorosi opened this issue Feb 19, 2025 · 2 comments · Fixed by #3676
Assignees
Labels
completed This item is complete and has been merged/shipped feature-request This item refers to a feature request for an existing or new utility logger This item relates to the Logger Utility

Comments

@dreamorosi
Copy link
Contributor

dreamorosi commented Feb 19, 2025

Use case

In #3617 we have implemented a mechanism to buffer logs. The next step in the implementation is to allow customers to flush the buffer whenever there's an uncaught error thrown in the handler scope. This will be an opt-in feature and this item focuses on implementing this behavior for the class method decorator.

Solution/User Experience

As mentioned, this will be an opt-in feature. Customers will be able to enable it by passing a new option to the decorator called flushBufferOnUncaughtError:

const logger = new Logger({
  logLevel: 'WARN',
  bufferConfig: {
    maxBytes: 1024,
  }
});

class Lambda {
  @logger.injectLambdaContext({
    flushBufferOnUncaughtError: true, // default is false
  })
  async handler(event: unknown) {
    logger.debug('I am a debug'); // buffered
    logger.info('I am an info'); // NOT buffered
    
    throw new Error('I am an error'); // this causes the buffer to be flushed
  }
}

In terms of implementation, we'll have to refactor the current decorator implementation here.

Contrary to the ones in #3633, the changes for the decorator should be fairly straightforward. Currently we have a try/catch block in which we are actually not handling the catch block, but only the finally one:

let result: unknown;
try {
  result = await originalMethod.apply(this, [event, context, callback]);
} finally {
  if (options?.clearState || options?.resetKeys) loggerRef.resetKeys();
}

return result;

we should refactor this to something like:

try {
  return await originalMethod.apply(this, [event, context, callback]);
} catch (error) {
  // if log buffering is enabled, flush the buffer here
} finally {
  if (options?.clearState || options?.resetKeys) loggerRef.resetKeys();
}

In order to add the new config option to the decorator, you'll have to add it to the corresponding type here. Depending on whether you work on this or #3633 first, you might already have the type available.

Other than that, we should make sure the new code is covered by unit tests and the new type in the options is documented with comments.

Alternative solutions

N/A

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

@dreamorosi dreamorosi added feature-request This item refers to a feature request for an existing or new utility logger This item relates to the Logger Utility on-hold This item is on-hold and will be revisited in the future labels Feb 19, 2025
@dreamorosi dreamorosi moved this from Triage to On hold in Powertools for AWS Lambda (TypeScript) Feb 21, 2025
@dreamorosi dreamorosi self-assigned this Feb 28, 2025
@dreamorosi dreamorosi added confirmed The scope is clear, ready for implementation and removed on-hold This item is on-hold and will be revisited in the future labels Feb 28, 2025
@dreamorosi dreamorosi moved this from On hold to Working on it in Powertools for AWS Lambda (TypeScript) Feb 28, 2025
@github-project-automation github-project-automation bot moved this from Working on it to Coming soon in Powertools for AWS Lambda (TypeScript) Feb 28, 2025
Copy link
Contributor

⚠️ COMMENT VISIBILITY WARNING ⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

@github-actions github-actions bot added pending-release This item has been merged and will be released soon and removed confirmed The scope is clear, ready for implementation labels Feb 28, 2025
Copy link
Contributor

github-actions bot commented Mar 7, 2025

This is now released under v2.16.0 version!

@github-actions github-actions bot added completed This item is complete and has been merged/shipped and removed pending-release This item has been merged and will be released soon labels Mar 7, 2025
@dreamorosi dreamorosi moved this from Coming soon to Shipped in Powertools for AWS Lambda (TypeScript) Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed This item is complete and has been merged/shipped feature-request This item refers to a feature request for an existing or new utility logger This item relates to the Logger Utility
Projects
Development

Successfully merging a pull request may close this issue.

1 participant