Skip to content

Logging/log emitter - #90

Open
Prateep07 wants to merge 6 commits into
nitrocloudofficial:developfrom
Prateep07:logging/log-emitter
Open

Prateep07 wants to merge 6 commits into
nitrocloudofficial:developfrom
Prateep07:logging/log-emitter

Conversation

@Prateep07

@Prateep07 Prateep07 commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Add a process-wide log_emitter with subscribe(callback) -> unsubscribe.
  • Forward structured records from NitroStack loggers to subscribers, including level, message, timestamp, logger name, and extra fields.
  • Deliver records through bounded per-subscriber buffers so slow callbacks don’t block logging. Drop the oldest queued record when a buffer is full.
  • Isolate subscriber exceptions and stop delivery on unsubscribe.
  • Preserve FileLogger output configuration and capture HTTP logger records when propagation is disabled.

closes #72

Testing

  • Logging tests: passed.
  • Full suite: 834 passed, 4 failed.
  • An untouched copy of HEAD also had failures: 826 passed, 5 failed. The shared failures involved missing wheel (bdist_wheel), installed NitroStack metadata reporting 0.4.0 while pyproject.toml declares 0.4.2, and intermittent OAuth socket errors.
  • Fixed the HTTP logging test’s handler cleanup; it passes in the current run.

@AadarshM07

Copy link
Copy Markdown
Collaborator

Hi @Prateep07, thanks for the PR
Could you rebase this PR with the develop branch and remove the additional commit dependencies? Also, it would be great if you could follow the Closes #<issue-number> format in the description. This will link the issue and automatically close it when the PR is merged.

@Prateep07
Prateep07 force-pushed the logging/log-emitter branch from 75a6bf9 to 6ebf6ba Compare October 26, 2026 14:38

@AadarshM07 AadarshM07 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.

Changes looks good to me, tests also passes. we will need to run the pipeline once.

@manish-wekan

Copy link
Copy Markdown
Collaborator

@Prateep07
Thanks, the emitter works well: the full suite passes on develop, with no duplicate records, and unsubscribe and raising subscribers behave as #72 asks.

One fix before merge: install_log_handler() runs at import and adds a handler to the nitrostack logger, which stops Python's lastResort stderr fallback.

As a result, NitroStack warnings logged before the first FileLogger exists now disappear (e.g. No widget HTML found for route … shows on develop but not on this PR).

Please install the handler lazily: add it on the first subscribe(), remove it after the last unsubscribe, and add a test that a nitrostack warning still reaches stderr when nothing has subscribed.

~sent from claude-code

This branch has not been deployed

No deployments
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.

3 participants