Skip to content

Bug: decorated class methods cannot access this in Tracer #1056

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
dreamorosi opened this issue Aug 13, 2022 · 3 comments · Fixed by #1055
Closed

Bug: decorated class methods cannot access this in Tracer #1056

dreamorosi opened this issue Aug 13, 2022 · 3 comments · Fixed by #1055
Assignees
Labels
bug Something isn't working completed This item is complete and has been merged/shipped tracer This item relates to the Tracer Utility

Comments

@dreamorosi
Copy link
Contributor

dreamorosi commented Aug 13, 2022

Bug description

All three existing utilities (Logger, Tracer, Metrics) provide method decorators that customers can use to decorate a Lambda class in AWS Lambda. Decorators are an experimental feature in TypeScript and at the time of writing the only way of using them is to use Classes.

Here's a generic example of how customers are expected to use the Powertools decorators:

import { Tracer } from '@aws-lambda-powertools/tracer';
import { LambdaInterface } from '@aws-lambda-powertools/commons';

const tracer = new Tracer({ serviceName: 'serverlessAirline' });

class Lambda implements LambdaInterface {
    // Decorate your handler class method
    @tracer.captureLambdaHandler()
    public async handler(_event: any, _context: any): Promise<void> {
        /* ... */
    }
}

export const handlerClass = new Lambda();
export const handler = handlerClass.handler;

Every time the function is invoked, the Lambda service will call the exported handler, which is now decorated. While the example above shows a decorator that belongs to Tracer, what discussed below affects also the other two utilities as they all use a similar implementation.

According to the TypeScript documentation, a method decorator has the following signature/type annotation:

type MethodDecorator = <T>(
  target: Object,
  propertyKey: string | symbol,
  descriptor: TypedPropertyDescriptor<T>
) => TypedPropertyDescriptor<T> | void;

The parameters are:

  • target: can be either the constructor function of the class for a static member, or the prototype of the class for an instance member (in most of the cases that interest this discussion & the usage that we recommend, the latter applies).
  • propertyKey: name of the property (e.g. handler in the example above)
  • descriptor: property descriptor for the member

Let us now take a simplified & stripped down version of the current implementation seen across Powertools:

class DecoratorUtilClass {
  public decorate() {
    return function (
      target: any,
      propertyKey: string,
      descriptor: PropertyDescriptor
    ) {
      const originalMethod = descriptor.value;

      descriptor.value = (...args: any[]) => {
        // Do something before
        const result = originalMethod.call(target, args);
        // Do something after

        return result;
      };

      return descriptor;
    };
  }
}

The above would be used as follows:

const myUtilClass = new DecoratorUtilClass();

class Lambda implements LambdaInterface {
    // Decorate your handler class method
    @ myUtilClass.decorate()
    public async handler(_event: any, _context: any): Promise<void> {
        /* ... */
    }
}

export const handlerClass = new Lambda();
export const handler = handlerClass.handler;

The interesting bits of the implementation above are:

  • We save a reference of the original method being decorated const originalMethod = descriptor.value; [1]
  • We then reassign a new value to the descriptor to a custom function that:
    • Performs some actions before decorated method
    • Calls the decorated method by passing back the target as first parameter, hence returning the initial this (aka the decorated Class) (more on this in the "Possible solution" section)
    • Performs some actions after the decorated method has returned
  • Returns the descriptor with a new value (thus decorating it)

The issue with this implementation is that the current implementation prevents customers from accessing all attributes from the decorated class. This is because the implementation is using an () => {} arrow function and the incorrect value of this is being passed back down.

Additionally, there's another caveat that was ignored until now which is that when exporting the handler at the end of the file:

export const handlerClass = new Lambda();
export const handler = handlerClass.handler;

Customers actually should bind() the class back to the handler so that when used and called later it will have access to the correct value of this.

export const handlerClass = new Lambda();
export const handler = handlerClass.handler.bind(handlerClass);

[1] This is mostly harmless but the line const originalMethod = descriptor.value; is potentially useless. The reason why I say this is that description.value evaluates to undefined all the time. This happens because we most probably misunderstood the order of evaluation of a decorator (see "Order of Evaluation" section of this page). That section of the code is always evaluated when the utility is instantiated for the first time. In the context of Lambda, this means during the execution environment's initialisation (aka during cold start).

Further readings

Expected Behavior

Customers should potentially be able to decorate a Class method and then still be able to access both methods and attributes of the decorated class after the decoration has happened.

Current Behavior

Depending on the implementation & execution environment the behaviour varies (see possible solutions below).

Possible Solution

There are a few changes to the decorator implementation that essentially boil down to switching to a function() {} (instead of an () => {} arrow fn) and then passing the correct value for this:

class DecoratorUtilClass {
  public decorate() {
    return function (
      target: any,
      propertyKey: string,
      descriptor: PropertyDescriptor
    ) {
      const originalMethod = descriptor.value;

      // We save this reference so we can call other methods on the DecoratorUtilClass
      const tracerRef = this;

      // We change the function to a regular function instead of an arrow fn
      descriptor.value = (function (this: Handler, ...args: any[]) {
        
        // We save the reference to the decorated fn/handler (in the actual implementation this is needed)
        const handlerRef: Handler = this;
        
        // Do something before

        // We pass the handlerRef when calling/applying
        const result = originalMethod.call(handlerRef, args);
        // Do something after

        return result;
      };

      return descriptor;
    };
  }
}

This work is being done in #1055

Steps to Reproduce

import { Tracer } from '@aws-lambda-powertools/tracer';
import { LambdaInterface } from '@aws-lambda-powertools/commons';

const tracer = new Tracer({ serviceName: 'serverlessAirline' });

class Lambda implements LambdaInterface {
    myAttribute: string = "someValue";

    @tracer.captureLambdaHandler()
    public async handler(_event: any, _context: any): Promise<void> {
        /* ... */
        console.log(this.otherMethod);
    }

    public async otherMethod () {
      return this.myAttribute;
    }
}

export const handlerClass = new Lambda();
export const handler = handlerClass.handler;

Environment

  • Powertools version used: v1.1.0
  • Packaging format (Layers, npm): all
  • AWS Lambda function runtime: all
  • Debugging logs: N/A

Related issues, RFCs

#1026

@dreamorosi dreamorosi added bug Something isn't working tracer This item relates to the Tracer Utility labels Aug 13, 2022
@misterjoshua
Copy link
Contributor

@dreamorosi Thanks for your help fixing this issue so far.

@github-actions
Copy link
Contributor

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues 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 the pending-release This item has been merged and will be released soon label Aug 16, 2022
@dreamorosi
Copy link
Contributor Author

Released in v1.1.1

@dreamorosi dreamorosi removed the pending-release This item has been merged and will be released soon label Aug 18, 2022
@dreamorosi dreamorosi changed the title Bug (tracer): decorated class methods cannot access this Bug: decorated class methods cannot access this in Tracer Nov 14, 2022
@dreamorosi dreamorosi added the completed This item is complete and has been merged/shipped label Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working completed This item is complete and has been merged/shipped tracer This item relates to the Tracer Utility
Projects
None yet
2 participants