functions.logger.log crashes function if passed certain objects

[nikmartin]

[REQUIRED] Version info

node: 10

firebase-functions: 3.7.0

firebase-tools: 8.4.3

firebase-admin: 8.12.1

[REQUIRED] Test case

const functions = require('firebase-functions');
exports.loggerTest = functions.https.onCall((data, context) => {
    functions.logger.log('context:', context);
});

[REQUIRED] Steps to reproduce

I have two scenarios that will reliably crash a firebase function using the logger:

• log context in a callable
• log the entire request in an https function

[REQUIRED] Expected behavior

[REQUIRED] Actual behavior

1:18:40.769 PM execution took 3196 ms, finished with status code: 500 |  
  at func (/workspace/node_modules/firebase-functions/lib/providers/https.js:272:32) 
  at process._tickCallback (internal/process/next_tick.js:68:7) 
  at exports.assignCard.functions.https.onCall (/workspace/index.js:3591:20) 
  at Object.log (/workspace/node_modules/firebase-functions/lib/logger.js:45:5)
  at write (/workspace/node_modules/firebase-functions/lib/logger.js:12:84) 
  at JSON.stringify (<anonymous>) 
  Unhandled error TypeError: Converting circular structure to JSON

Were you able to successfully deploy your functions?

Yes

[nikmartin]

Obviously my logging the entire context object is probably incorrect, but it was this way for 2 years until i swapped out all console.log() for functions.logger.log() after upgrading to node 10

[nikmartin]

I attempted to fix and submit a PR, but I ran out of time. If it's not obvious, objects like requests on https functions, or context in callables have circular references in them, so using nodes util.inspect on them would likely solve the issue. The problem is I'm not great at mocha test writing, and I could not come up with a test for when util inspect returns an object reference named [Circular]. That's the object reference itself, NOT the string '[Circular]', so I couldn't get a test working in the short amount of time I allotted myself to work on it.

[mbleigh]

This is a little more complex than just using util.inspect since it's actually important to have valid JSON output for structured logging. We'll need to add circular reference detection to the logger, this is something I'll look into.

[rhodgkins]

@laurenzlong / @mbleigh can we get some movement on this issue please - #776 does fix it but a comment in that PR does raise a concern.