-
Notifications
You must be signed in to change notification settings - Fork 12.6k
chore(federation): fix error logs #37706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughThis PR standardizes error logging patterns across the federation-matrix package by converting logger.error calls from message-first to error-first argument ordering and replacing plain string logs with structured log objects containing contextual identifiers and message fields. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (7)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-10-28T16:53:42.761ZApplied to files:
📚 Learning: 2025-11-04T16:49:19.107ZApplied to files:
📚 Learning: 2025-09-19T15:15:04.642ZApplied to files:
🧬 Code graph analysis (4)ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
ee/packages/federation-matrix/src/events/member.ts (1)
ee/packages/federation-matrix/src/services/MatrixMediaService.ts (1)
ee/packages/federation-matrix/src/events/message.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
🔇 Additional comments (39)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5ffdd08 to
4301ee0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37706 +/- ##
===========================================
- Coverage 67.89% 67.78% -0.11%
===========================================
Files 3449 3449
Lines 114020 114030 +10
Branches 20956 20963 +7
===========================================
- Hits 77410 77292 -118
- Misses 34491 34627 +136
+ Partials 2119 2111 -8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
We were completely missing the errors the way we were logging errors.
See this code as an example of why the changes are needed:
This produces the following production logs:
as you can see this is what we were seeing:
"msg":"wrong error: {}"}Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.