Feature request: type Error | LogAttributes is not excepted as extra input

[ghdoergeloh]

Bug description

I would like to log a error inside a catch blog. In the newest version of Typescript a catched "Error" has the type unknown, because it could be anything. So I decided to use the following code:

    } catch (e) {
      logger.error('Error happened', e instanceof Error ? e : { error: e });
    }

but the type is not compatible.

Expected Behavior

The type should be accepted, because both variants are okay:

    } catch (e) {
      logger.error('Error happened', e as Error);
    }

    } catch (e) {
      logger.error('Error happened', { error: e });
    }

Current Behavior

The typescript compiler gives the following Message:

TS2345: Argument of type '[Error | { error: unknown; }]' is not assignable to parameter of type 'LogItemExtraInput'.
   Type '[Error | { error: unknown; }]' is not assignable to type '[string | Error]'.
     Type 'Error | { error: unknown; }' is not assignable to type 'string | Error'.
       Type '{ error: unknown; }' is not assignable to type 'string | Error'.
         Object literal may only specify known properties, and 'error' does not exist in type 'Error'.

Possible Solution

Changing the typedeclaration to the following worked:

declare type LogItemExtraInput = [Error | string | LogAttributes] | LogAttributes[];

Steps to Reproduce

import { Logger } from '@aws-lambda-powertools/logger';

export const logger = new Logger();
try {
  logger.info('Info');
} catch (e) {
  logger.error('Error happened', e instanceof Error ? e : { error: e });
}

Environment

• Powertools version used: 1.2.1
• Packaging format (Layers, npm): npm
• AWS Lambda function runtime: nodejs16
• Debugging logs:

Related issues, RFCs

[ghdoergeloh]

another option would be this type: [Error | string | LogAttributes,...LogAttributes] | LogAttributes[]

But trying around a bit I found the solution, that is even shorter and does exactly the same:

If e is an Error, the result is the same:

    } catch (e) {
      logger.error('Error happened', e as Error);
      logger.error('Error happened', { error: e });
    }

both will have an error property: "error":{"name":"...","stack":"...

[dreamorosi]

Hi @ghdoergeloh, thank you for reporting this.

Please allow us some time to go through the repro example and return to you with a reply.

[ijemmy]

@ghdoergeloh

Could you tell us on why do you want to pass the { error: e } instead of e? More specifically, why do you want to throw a "non-error" object in the code?

Logger assumes that e always be an Error object. In that case, Logger will put it in the error field automatically.

While we can throw anything, but that is not a recommended practice.

[ghdoergeloh]

yeah, that's right, but i cannot guarantee that a lib i am using is alsways throwing only errors and if it is something different (e.g. jest overwrites the default Error object, which causes "instanceof Error" to fail - see jestjs/jest#2549) then I won't get the complete information about the error in the log.

So I don't want to throw it, I want to catch and log it.

[ijemmy]

@ghdoergeloh

Would if work if you reconstruct the error object like this?

logger.error('Error happened', e instanceof Error ? e : new Error({...}) );

I understand that this will be more lengthy. But I'm reluctant to widen the interface because other tools throw non-error object. If we accept LogAttributes type, the e could be any key-value object. That fails the promise of strong typing.

Also, the type LogItemExtraInput is also reused by logger.debug(), logger.info(), and logger.warn(). Other methods will accept extra key-value inputs. If we want to avoid this side effects, method error() must have a different set of parameters.

---

From another perspective, we can say that the extra value of logger.xxx() should be able to take any arbitrary key-value mapping as an extra input. If there is a significant numbers of +1, we can consider widen the type.

---

@saragerion What's your thought on this?

[dreamorosi]

As discussed above, using logger.error and passing the error directly (i.e. logger.error('This is the first error', error as Error); will already result in a log like this:

{
    "level": "ERROR",
    "message": "This is the first error",
    "service": "serverlessAirline",
    "timestamp": "2021-12-12T22:12:39.345Z",
    "xray_trace_id": "abcdef123456abcdef123456abcdef123456",
    "error": {
        "name": "Error",
        "location": "/path/to/my/source-code/my-service/handler.ts:18",
        "message": "Unexpected error #1",
        "stack": "Error: Unexpected error #1    at lambdaHandler (/path/to/my/source-code/my-service/handler.ts:18:11)    at Object.<anonymous> (/path/to/my/source-code/my-service/handler.ts:35:1)    at Module._compile (node:internal/modules/cjs/loader:1108:14)    at Module.m._compile (/path/to/my/source-code/node_modules/ts-node/src/index.ts:1371:23)    at Module._extensions..js (node:internal/modules/cjs/loader:1137:10)    at Object.require.extensions.<computed> [as .ts] (/path/to/my/source-code/node_modules/ts-node/src/index.ts:1374:12)    at Module.load (node:internal/modules/cjs/loader:973:32)    at Function.Module._load (node:internal/modules/cjs/loader:813:14)    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:12)    at main (/path/to/my/source-code/node_modules/ts-node/src/bin.ts:331:12)"
    }
}

Furthermore, it's also possible to customize the key used for the error by using logger.error('This is the second error', { myCustomErrorKey: error as Error } );.

In regards to custom errors, as long as these errors extend the Error object, Logger shouldn't complain:

import { Logger, injectLambdaContext } from "@aws-lambda-powertools/logger";
import middy from "@middy/core";

const logger = new Logger({ serviceName: "someName" });

class MyCustomError extends Error {
  public foo: string;

  constructor (message: string) {
    super(message)
    this.foo = "bar"
  }
}

export const handler = middy(async () => {
  try {
    throw new MyCustomError("error");
  } catch (err) {
    if (err instanceof MyCustomError) {
      logger.error("some error", err);
    }
  }
}).use(injectLambdaContext(logger));

At the moment we are not going to widen the type for errors as I'm inclined to consider this more akin to a feature request rather than a bug.

I'm going to close this issue, however I'm open to revisit the decision in case there's enough customer demand.

[github-actions]

⚠️ 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.

[phawxby]

@dreamorosi to throw in my 2p the type definition does seem a little weird and should probably be more like

type LogItemExtraInput = [Error | string | LogAttributes][];

It seems a little confusing that elsewhere you can pass in as many additional objects as you like to help with debugging, including with .error(), but if one of those objects is an error or string it breaks the typings. If this is a feature request can it be reopened as such?

[dreamorosi]

Hi @phawxby, thank you for the message and apologies for the delay.

If I understand your comment correctly the best place to continue the discussion would be under #1777. Please feel free to complement the issue with your point of view.