firebase / firebase-functions

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

[logger]: express deprecated req.host: Use req.hostname instead #1569

Closed jdcoldsmith closed 2 months ago

jdcoldsmith commented 4 months ago

Related issues

[REQUIRED] Version info

4.9.0 (I realize that this isn't the latest version but I have compared the code and couldn't find anything related to logger)

node:

20

firebase-functions:

Hello, I recently experienced this error in a 2nd gen Node.js 20 firebase function and I wanted to pass it along to you fine folks. image This seems to be an issue with the logger package.

google-oss-bot commented 4 months ago

I found a few problems with this issue:

exaby73 commented 3 months ago

Hey @jdcoldsmith. Could you give me a code sample that causes this log?

jdcoldsmith commented 3 months ago

It seems to happen for me whenever I call log within an onRequest firebase function. This is how I import log: const { log } = require('firebase-functions/logger');

taeold commented 2 months ago

Hi @jdcoldsmith. We reviewed our source code and don't see any direct reference to the req.host object.

I'm wondering what object you are passing to the logger module. The logger module will iterate through all keys in an object that is passed while serializing the object, and maybe the req.host is being used within that code.

jdcoldsmith commented 2 months ago

Ahh ok I think I see what could be happening. Whenever I get an error in my function I am passing the entire express request to the log function of logger (e.g. log('Error', { request })). Is it possible that logger is throwing this warning from that?

exaby73 commented 2 months ago

Hey @jdcoldsmith. Yeah this is entirely possible since the logger iterates over the entire object to format it correctly before actually logging it. I'd recommend you log only properties you're interested in, like the hostname, any query params, etc. instead of logging the entire request object which may be logging a lot of unnecessary information polluting your logs.

But since this is not an issue with the logger itself, I feel safe to close this issue. Thank you!

jdcoldsmith commented 2 months ago

Ok that makes sense! Thank you!