Skip to content

fix (full-sync): Adding pino logger for probot update#961

Open
dolan-a wants to merge 7 commits into
github:main-enterprisefrom
dolan-a:hotfix/full-sync-probot-v14
Open

fix (full-sync): Adding pino logger for probot update#961
dolan-a wants to merge 7 commits into
github:main-enterprisefrom
dolan-a:hotfix/full-sync-probot-v14

Conversation

@dolan-a

@dolan-a dolan-a commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

With the Probot v14 update, it seems that logging in full-sync no longer works:

$ node --env-file=.env full-sync.js                               
Fatal error during full sync: TypeError: Cannot read properties of null (reading 'info')
    at performFullSync (/home/dol/dev/personal/safe-settings/full-sync.js:7:14)
    at Object.<anonymous> (/home/dol/dev/personal/safe-settings/full-sync.js:25:1)
    at Module._compile (node:internal/modules/cjs/loader:1761:14)
    at Object..js (node:internal/modules/cjs/loader:1893:10)
    at Module.load (node:internal/modules/cjs/loader:1481:32)
    at Module._load (node:internal/modules/cjs/loader:1300:12)
    at TracingChannel.traceSync (node:diagnostics_channel:328:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:245:24)
    at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5)
    at node:internal/main/run_main_module:33:47

This PR has the following changes:

  1. Adds a logger (via already-installed pino)
  2. Adds a few unit tests to confirm performFullSync in full-sync runs okay (test fails without the logger change; passes with it)
@dolan-a

dolan-a commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@decyjphr - can we get this considered for an upcoming release?

Copilot AI left a comment

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.

Pull request overview

This PR updates the full-sync CLI entrypoint to restore logging compatibility after the Probot v14 upgrade by explicitly providing a logger to createProbot, and adds unit tests around performFullSync to prevent regressions.

Changes:

  • Create a pino logger and pass it to createProbot via overrides.log to avoid probot.log being null.
  • Add a null-guard around settings.errors to avoid crashing when syncInstallation() returns null.
  • Export performFullSync and add unit tests for logger wiring and null-safety.
Show a summary per file
File Description
full-sync.js Adds a pino logger override for Probot v14 and exports performFullSync for testing.
test/unit/full-sync.test.js Introduces unit tests for performFullSync (logger override + null settings).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 3
Comment thread full-sync.js
Comment thread test/unit/full-sync.test.js Outdated
Comment thread test/unit/full-sync.test.js Outdated

@decyjphr decyjphr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@dolan-a Thanks for the changes. I used Copilot Code review and it suggested a few changes. Could you review and add them?

@marcusburghardt

Copy link
Copy Markdown

It seems also related to #955

@dolan-a

dolan-a commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

@decyjphr - Thanks, this is ready for another review

@dolan-a dolan-a requested a review from decyjphr June 27, 2026 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants