[FIX] Update visitor info on email reception based on current inbox settings#23280
[FIX] Update visitor info on email reception based on current inbox settings#23280
Conversation
sampaiodiego
left a comment
sampaiodiego
left a comment
There was a problem hiding this comment.
Although it is already merged some changes would be good to do.
|
|
||
| for await (const emailInboxRecord of emailInboxesCursor) { | ||
| console.log('Setting up email interceptor for', emailInboxRecord.email); | ||
| logger.info('Setting up email interceptor for', emailInboxRecord.email); |
There was a problem hiding this comment.
this usage will be deprecated (we need to add a warning for that)
please log either an object or use placeholders: https://github.com/pinojs/pino/blob/master/docs/api.md#message-string
| } catch (e) { | ||
| } catch (e: any) { | ||
| // In case the email message history has been received by other instance.. | ||
| logger.error(e.message); |
There was a problem hiding this comment.
if you have an error object, you can log it entirely
logger.error(e);
if for some reason TS complains about it, you can add the error object to the err key
logger.error({ err: e });
| }); | ||
| }).catch((error) => { | ||
| Livechat.logger.log('Error receiving Email: %s', error.message); | ||
| console.log(error); |
There was a problem hiding this comment.
please avoid using console.log
| }).catch((error) => { | ||
| Livechat.logger.log('Error receiving Email: %s', error.message); | ||
| console.log(error); | ||
| Livechat.logger.error('Error receiving Email: %s', error.message); |
There was a problem hiding this comment.
same here with logging the error object. if you need to add a message to the log, you can add the msg key:
Livechat.logger.error({
msg: 'Error receiving Email',
err: error,
});
| if (guest) { | ||
| logger.debug(`Guest with email ${ email } found with id ${ guest._id }`); | ||
| if (guest.department !== department) { | ||
| logger.debug(`Switching departments for guest ${ guest._id }. Previous: ${ guest.department } New: ${ department }`); |
There was a problem hiding this comment.
for debugging purposes might be useful to add extra data, for this it is useful logging big objects with an additional message, like:
logger.debug({
msg: 'Switching departments for guest',
guest,
new: department,
});| let room = LivechatRooms.findOneByVisitorTokenAndEmailThread(guest.token, thread, {}); | ||
| logger.debug(`Guest ${ guest._id } obtained. Attempting to find or create a room on department ${ department }`); | ||
| let room = LivechatRooms.findOneByVisitorTokenAndEmailThreadAndDepartment(guest.token, thread, department, {}); | ||
| logger.debug(`Room ${ room?._id } found for guest ${ guest._id }`); |
There was a problem hiding this comment.
same here with debugging info:
logger.debug({
msg: 'Room found for guest',
room,
guest,
});
|
Hey @sampaiodiego , i wasn't aware of those new conventions for logs, would be good to create a "log documentation" page somewhere to describe conventions and best practices, wdyt? 👀 |
definitely a good idea.. I'll try to draft something 👍 |
Co-authored-by: Murtaza Patrawala <34130764+murtaza98@users.noreply.github.com>
3 participants
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments