Skip to content

Error handling for malformed session file #38

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

Conversation

eliasyishak
Copy link
Contributor

@eliasyishak eliasyishak commented Mar 16, 2023

Fixes: #20

Adding functionality here to not crash the dart process incase the user somehow edited the .dart/session.json file that contains information about sessions across all dart related tooling for Google Analytics. Below is an example of what a valid session file looks like

{"session_id":794250000000,"last_ping":794250000000}

session_id: the current session value
last_ping: the timestamp for the last event that was sent

Both values above are milliseconds since epoch time.

session_id will get updated to be last_ping if the time difference between the two is greater than the duration allotted for a session (30 mins atm)

If the user somehow changes the contents of this file though, any dart and flutter related tool will also crash. As a result, this PR aims to detect the file has been malformed, recreate it, and parse that file again.

@eliasyishak eliasyishak changed the title 20 error handling for malformed session file Error handling for malformed session file Mar 16, 2023
int _sessionId;
int _lastPing;

// Initialize as 0 but will get parsed to real values
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@christopherfujino i wanted to avoid using the late keyword here for these 2 variables, let me know what you think

Copy link
Contributor

Choose a reason for hiding this comment

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

If we're calculating this within the private constructor, can we not calculate it in the factory?

And if not, can we go the other way and get rid of the factory altogether?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an instance method _refreshSessionData() that will be used to calculate those two ints so it can't be used within the factory constructor body.

But I agree with removing the factory constructor

Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why using avoid late here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was referring to the usage guide where it says to avoid late where possible. I also have used it in other parts of the package that could probably be refactored once this package is stable and being used

https://dart.dev/guides/language/effective-dart/usage#avoid-late-variables-if-you-need-to-check-whether-they-are-initialized

Copy link
Contributor

@andrewkolos andrewkolos Mar 27, 2023

Choose a reason for hiding this comment

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

From the link you shared:

AVOID late variables if you need to check whether they are initialized

I think using late is fine if a variable should definitely be initialized before it's read. While I believe avoiding late by default is good, I think initializing variables with placeholder values has more (potential) problems. If we intend for a variable to get "properly" initialized before first read, then reading it before that initialization should throw, and late does this for us.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great point, I also agree with that. I'll revert that back to use late again

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@andrewkolos andrewkolos left a comment

Choose a reason for hiding this comment

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

LGTM with nit

@eliasyishak eliasyishak merged commit ba1c2fd into dart-lang:main Mar 28, 2023
@eliasyishak eliasyishak deleted the 20-error-handling-for-malformed-session-file branch March 28, 2023 15:41
mosuem pushed a commit that referenced this pull request Aug 13, 2024
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.0 to 3.5.2.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@8f4b7f8...8e5e7e5)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add error handling for malformed session json file
4 participants