-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: forgive timestamps provided as ms/μs #13
base: dev
Are you sure you want to change the base?
Conversation
I think that this change more properly belongs in the binding code for nodejs, so that what libsession-nodejs would expose is a function to nodejs that takes millisecond input, and produces millisecond output, as those are what native nodejs tools expect, and does the ÷1000 or ×1000 in the function wrapper. Milliseconds unix timestamps are a non-common concept in general; pretty much every other language defines timestamps in terms of seconds: plain integer seconds (older C apis, and languages that adopted that convention), a seconds/nanosecond pair (newer C APIs, Java), or a floating point seconds value (Python, Swift, and loads of other languages). Older Java used milliseconds, but when they revamped the date time API to fix various limitations in it a decade ago they moved away from it. That's not to say all language should be the same, by any means, but it's the job of the specific language binding code to pave over conceptual differences in what the specific language wants, rather than make the library worry about possible different language conventions. |
This is unfortunately less of a nodejs-specific thing and more an attempt to fix some We could look at implementing this timestamp conversion fix across all 3 platforms or wrappers but I guess the arguments for doing it in
Since some users will already have configs which have invalid timestamps just throwing when given an invalid value wouldn't help those users and may result in unexpected crashes (we introduced a crash on Desktop when originally adding exceptions for the If we are able to get some more time to dedicate to |
I see. Yeah, if that broken data is out there in the wild then we need to deal with it, and here is the place that makes sense (sort of like the protobuf double-wrapping issue). Are current (or in-progress) clients now passing the correct value, so that we can treat this as a workaround for past issues?
100% yes, and that is indeed on the agenda for starting right away in 2025. |
No description provided.