Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

Conversation

@marandaneto
Copy link
Contributor

@marandaneto marandaneto commented Aug 3, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

fix: converting UTC and ISO timestamp when missing Locale/TimeZone do not error

💡 Motivation and Context

Andromax G36C1H
Smartfren Andromax AD6B1H
Andromax C46B2H

devices have problems with getting US locale or UTC/GTM time zones.

💚 How did you test it?

I can't really test it, I don't have those devices nor found emulators.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

🔮 Next steps

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Follow up PR (after hackweek for example) would be to handle when date is null. We should't do sessions at all at least in that case


duration = calculateDurationTime(this.timestamp);
sequence = getSequenceTimestamp(this.timestamp);
if (this.timestamp != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Likely worth not having sessions enabled at all for this device since they'll be broken

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, SessionAdapter will drop on deserialization at least.

@marandaneto marandaneto requested a review from a team August 3, 2020 15:16
@marandaneto marandaneto changed the title fix: converting UTC and ISO timestamp if missing Locale fix: converting UTC and ISO timestamp when missing Locale do not error Aug 3, 2020
@marandaneto marandaneto changed the title fix: converting UTC and ISO timestamp when missing Locale do not error fix: converting UTC and ISO timestamp when missing Locale/TimeZone do not error Aug 4, 2020
@marandaneto marandaneto merged commit 47c1f7f into getsentry:master Aug 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants