Skip to content

feat: [Fireflies] log cleanly#15618

Merged
charlesBochet merged 2 commits intotwentyhq:mainfrom
alex-galey:dev
Nov 5, 2025
Merged

feat: [Fireflies] log cleanly#15618
charlesBochet merged 2 commits intotwentyhq:mainfrom
alex-galey:dev

Conversation

@alex-galey
Copy link
Copy Markdown
Contributor

[0.2.2] - 2025-11-04

Added

  • Enhanced logging system: Introduced configurable AppLogger class with log level support (debug, info, warn, error, silent)
    • Environment-based log level configuration via LOG_LEVEL environment variable
    • Test environment detection to prevent log noise during testing
    • Context-aware logging with proper prefixes for better debugging
  • Improved error handling: Enhanced webhook signature verification with detailed debug logging
  • Better debugging capabilities: Added comprehensive logging throughout webhook processing pipeline

Enhanced

  • Webhook signature verification: Improved signature validation with detailed logging for troubleshooting
  • Error messages: More descriptive error logging for failed operations and security violations
  • Development experience: Better debugging information for webhook processing and API interactions

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR introduces a configurable logging system for the Fireflies integration, replacing scattered console.log and console.error calls with a centralized AppLogger class.

Key Changes

  • Implemented AppLogger class with configurable log levels (debug, info, warn, error, silent, critical)
  • Replaced DEBUG_LOGS boolean environment variable with LOG_LEVEL enum for granular control
  • Added automatic test environment detection to prevent log noise during testing
  • Enhanced error messages with context-aware logging prefixes
  • Improved participant name extraction logic in fireflies-api-client.ts

Issues Found

  • Critical: The debug array in WebhookHandler is no longer populated after removing the logDebug/logError methods, but is still returned in result.debug. This will always be empty now.

Confidence Score: 4/5

  • This PR is mostly safe to merge with one minor issue to address
  • The logging refactoring is well-implemented with proper log levels and test environment handling. However, the debug array in WebhookHandler is no longer populated, which could break functionality that depends on result.debug. This is a minor issue that should be fixed before merging.
  • Pay attention to webhook-handler.ts - the debug array needs to be either removed or properly populated

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-apps/hacktoberfest-2025/fireflies/serverlessFunctions/receive-fireflies-notes/src/webhook-handler.ts 3/5 Replaced old logging methods with new logger system, but debug array is no longer populated
packages/twenty-apps/hacktoberfest-2025/fireflies/serverlessFunctions/receive-fireflies-notes/src/logger.ts 5/5 New logger implementation with configurable log levels and test environment detection - looks good
packages/twenty-apps/hacktoberfest-2025/fireflies/serverlessFunctions/receive-fireflies-notes/src/fireflies-api-client.ts 5/5 Replaced console.log/error with logger calls, improved name extraction logic in participant parsing

Sequence Diagram

sequenceDiagram
    participant WH as WebhookHandler
    participant Logger as AppLogger
    participant FA as FirefliesApiClient
    participant TC as TwentyCrmService
    participant ENV as Environment

    WH->>Logger: createLogger('fireflies')
    Logger->>ENV: Check LOG_LEVEL & NODE_ENV
    ENV-->>Logger: Return config
    
    WH->>Logger: debug('invoked')
    Logger->>Logger: shouldLog('debug')?
    alt LOG_LEVEL allows debug
        Logger->>Console: console.log('[fireflies] invoked')
    else LOG_LEVEL too high or test env
        Logger-->>WH: (suppressed)
    end
    
    WH->>FA: new FirefliesApiClient(apiKey)
    FA->>Logger: createLogger('fireflies-api')
    
    WH->>FA: fetchMeetingDataWithRetry()
    FA->>Logger: debug('fetching meeting...')
    FA-->>WH: meetingData
    
    WH->>TC: new TwentyCrmService(apiKey)
    TC->>Logger: createLogger('20 CRM Service')
    
    WH->>TC: matchParticipantsToContacts()
    TC->>Logger: warn('Duplicate email...')
    TC-->>WH: matched contacts
    
    WH->>TC: createMeeting()
    TC->>Logger: debug('createMeeting variables')
    TC-->>WH: meetingId
    
    alt Error occurs
        WH->>Logger: error('error: message')
        Logger->>Console: console.error('[fireflies] error: message')
    end
Loading

9 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines 18 to 19
private debug: string[] = [];
private isTestEnvironment: boolean;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: The debug array is no longer populated after removing logDebug and logError methods. result.debug will always be empty now. Consider removing this field entirely or add logic to populate it with logger messages.

Suggested change
private debug: string[] = [];
private isTestEnvironment: boolean;
// Remove this field if debug tracking is no longer needed
// private debug: string[] = [];
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-apps/hacktoberfest-2025/fireflies/serverlessFunctions/receive-fireflies-notes/src/webhook-handler.ts
Line: 18:19

Comment:
**logic:** The `debug` array is no longer populated after removing `logDebug` and `logError` methods. `result.debug` will always be empty now. Consider removing this field entirely or add logic to populate it with logger messages.

```suggestion
  // Remove this field if debug tracking is no longer needed
  // private debug: string[] = [];
```

How can I resolve this? If you propose a fix, please make it concise.

@charlesBochet charlesBochet merged commit 583d490 into twentyhq:main Nov 5, 2025
38 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 5, 2025

Thanks @alex-galey for your contribution!
This marks your 2nd PR on the repo. You're top 15% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants