-
Notifications
You must be signed in to change notification settings - Fork 246
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
Add FailedToParse timeline items #1224
Conversation
Codecov ReportBase: 74.41% // Head: 74.42% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1224 +/- ##
=======================================
Coverage 74.41% 74.42%
=======================================
Files 112 113 +1
Lines 12869 12952 +83
=======================================
+ Hits 9577 9639 +62
- Misses 3292 3313 +21
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
367b025
to
c58d3b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
c58d3b0
to
1b5b3b7
Compare
When a timeline event comes in but we fail to deserialize it, previously we would just ignore it. Now, if we can still parse everything except the content, we add an
EventTimelineItem
with a new kind of content so the fact an event arrived isn't just silently discarded. The raw event and error can be reflected in the UI somehow. If something outsidecontent
fails to parse, that is either a bug in the server or in our code and we just log an error.The first case can easily happen by having a buggy or malicious client, so I think it's important we preserve as much information as possible. In the latter case, just logging an error should be fine as it's much less likely (requires a bug in the server, ruma or the SDK).