firebase / firebase-functions

Firebase SDK for Cloud Functions
https://firebase.google.com/docs/functions/
MIT License
1.02k stars 201 forks source link

Logging Error object has a difficult to read output #773

Open shalomhalbert opened 3 years ago

shalomhalbert commented 3 years ago

Related issues

[REQUIRED] Version info

node: 10.22.0

firebase-functions: 3.11.0

firebase-tools: 8.9.0

firebase-admin: 8.13.0

[REQUIRED] Test case

const firebaseFunctions = require('firebase-functions');
try {
     throw new Error('Message from the thrown error.');
 } catch (err) {
     firebaseFunctions.logger.error('Logged message', err);
 }

[REQUIRED] Steps to reproduce

Run the test case. For example:

const firebaseFunctions = require('firebase-functions');

it('Test', () => {
  try {
       throw new Error('Message from the thrown error.');
   } catch (err) {
       firebaseFunctions.logger.error('Logged message', err);
   }
});

[REQUIRED] Expected behavior

Something like:

{
   "severity":"ERROR",
   "message":"Logged message"
   "stack":"Error: Message from the thrown error.\n   <insert stack>"
}

[REQUIRED] Actual behavior

{
   "severity":"ERROR",
   "message":"Logged message Error: Message from the thrown error.\n   <insert stack>"
}

Were you able to successfully deploy your functions?

Yes, it deploys fine. This is about logging errors in a way that is easier to read.

The actual output isn't ideal because the the logged message ("Logged message ") is concatenated with the Error object's message ("Message from the thrown error.") and stack. It's clear this happens because Error isn't a simple object, which means the logger won't interpret it as a "jsonPayload". However, this makes reading errors more difficult. Is there a way to log errors so that the output is more like the following?

This is just the tip of the iceberg, because I'd like to create custom error classes that extends the Error class, but those print just as poorly.

google-oss-bot commented 3 years ago

I found a few problems with this issue:

jankalthoefer commented 3 years ago

I have the same problem for objects I pass to the logger. So e.g. if I log the following:

import { logger } from 'firebase-functions';

const nestedObject = 
{
    layer: '1',
    next: {
        layer: '2',
        next: {
                layer: '3',   
        }
    }
};

logger.log('Simple Object:', nestedObject);

The output I get is: {"layer":"1","next":{"layer":"2","next":{"layer":"3","next":{"layer":"4"}}},"severity":"INFO","message":"Simple Object:"}

For more complex objects the output is almost unreadable.

taeold commented 3 years ago

Thanks for reporting the issue @shalbert94 and @jankalthoefer.

A workaround would be to wrap the error stack inside a simple object:

const functions = require('firebase-functions');
try {
     throw new Error('Message from the thrown error.');
 } catch (err) {
     functions.logger.error('Logged message', { stack: err.stack });
 }

I suspect that this isn't a satisfying answer.

FWIW, you can always use a custom logger that better fits your use case. functions.logger module exists to make logs entries emitted from Cloud Functions look good in Google's Cloud Logging, but you are absolutely freed to choose any other logging library (no guarantee that it will work w/ Cloud Logging though).

shalomhalbert commented 3 years ago

For anyone who'd like a potentially more complete solution, the following module is what I've come up with.

const firebaseFunctions = require('firebase-functions');

const errorToMetadata = (err) => {
    return {
        metadata: {
            ...err,
            message: err.message,
            stack: err.stack
        }
    };
};

const formatMetadata = (payload) => {
    if (payload instanceof Error) {
        return errorToMetadata(payload);
    }

    const payloadType = typeof payload;
    if (payloadType === 'object') {
        return { metadata: { ...payload } };
    }

    return { metadata: payload };
};

const info = (message, data) => {
    const formattedMetadata = formatMetadata(data);

    firebaseFunctions.logger.info(message, formattedMetadata);
};

const error = (message, data) => {
    const formattedMetadata = formatMetadata(data);

    firebaseFunctions.logger.error(message, formattedMetadata);
};

exports.info = info;
exports.error = error;

Explanation:

Because of the API's implementation, ES6 classes are not converted to a jsonPayload, unlike plain objects. That's where

const payloadType = typeof payload;
if (payloadType === 'object')

comes in. I couldn't find a good approach for identifying ES6 classes, as opposed to plain objects, so instead, it made sense to just clone enumerable properties of every object.

Opinions:

  1. firebaseFunctions.logger.error should cleanly log an Error.
  2. An ES6 class should be an acceptable jsonPayload

Edit: Replaced one solution with another I think is better

taeold commented 3 years ago

@shalbert94 Thanks for making the suggestions. I think that makes for a good FR. I don't think we can prioritize the work ATM, but feel free to check in with us if you have some time to contribute.

j-frost commented 3 years ago

I thought I was taking crazy pills. Thanks for the workaround @shalbert94 . In my case, errors would just go missing and show up as empty objects - but only sometimes. Took me quite a number of hours to find this issue.