Skip to content
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

node process does not reflect timezone changes #33805

Closed
aghArdeshir opened this issue Jun 9, 2020 · 8 comments
Closed

node process does not reflect timezone changes #33805

aghArdeshir opened this issue Jun 9, 2020 · 8 comments
Labels
feature request Issues that request new features to be added to Node.js.

Comments

@aghArdeshir
Copy link

  • Version: 12.17.0
  • Platform: Linux 5.4.0-33-generic # 37-Ubuntu SMP Thu May 21 12:53:59 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
  • Platform: Linux 3.16.0-4.9-amd64 # 1 SMP Debian 3.16.7-ckt25-1 (2018-11-21) x86_64 GNU/Linux
    (it reproduces on both platforms)
  • Subsystem: ?

What steps will reproduce the bug?

  • open two terminals locally on machine
  • run node on terminal 1
  • new Date().getTimezoneOffset()
  • change timezone in terminal 2
  • ask for new Date().getTimezoneOffset() in node in terminal 1 again
  • note that timezone change is not reflected
  • exit node process in terminal 1 and enter node again (another node process)
  • now new Date().getTimezoneOffset() reflects the new timezone
  • note that this not-reflecting-new-timezone thing affects creating new dates and any other date operations. getTimezoneOffset() is just a sample

How often does it reproduce? Is there a required condition?

always reproduces, no required condition.

What is the expected behavior?

To reflect and update timezone of node process, when timezone of system is changed.

What do you see instead?

I see the old timezone.

Additional information

This bug has been created and reported previously, as I read, some people mentioned that the main bug is with V8, and linked to some bugs there. By the way, all previous reported bugs were closed. I link to everything I found here:
#4022
#20026
#19974
#3449

I created this issue as some time has passed and honestly, I couldn't find out why the previous issues were closed, and V8 discussions were beyond my knowledge.

What is the matter?

We have a system, a rather big one, composed of different programming languages and technologies, all components of our system works fine when timezone changes, except nodejs. There are solutions like https://github.com/evanlucas/reset-date-cache, but suppose node queries sql queries and created date objects from timestamps in sql tables, and timezone needs to be kept updated. the only way to keep it updated is to reset cache on each page request, which as noted in https://github.com/evanlucas/reset-date-cache:

The underlying call being made is quite expensive so it should only be used where absolutely necessary.

Screenshot of reproduction

Screenshot from 2020-06-09 18-58-34

@jasnell jasnell added the feature request Issues that request new features to be added to Node.js. label Jun 9, 2020
@jasnell
Copy link
Member

jasnell commented Jun 9, 2020

As far as I understand it, It's not so much a bug in v8 as it's a performance optimization. As @evanlucas notes in the reset-date-cache, it's a fairly expensive operation to be doing continually. But, that said, I can't see much reason why we can't incorporate reset-date-cache into core as part of the v8 module. It would still require you to call it regularly to catch updates (although, I do find it odd that a system would be changing system timezones that frequently).

@aghArdeshir
Copy link
Author

OK! lets think a system does not change time-zone regularly, but only one time in it's whole lifecycle.
As a js/nodejs developer, I, shouldn't have to setup system calls to catch that one-time time-zone change in system, and send the event somehow to nodejs and then inside nodejs catch the event and reset date cache for that one-time time-zone change.

I think it's more appropriate to say, OK, I use nodejs, its a standard and stable runtime/framework/programming language and widely used and should have a stable date module and like every other technology I trust in internals. (like I said, among tens of technologies we use, its only nodejs that is problematic)

Also, this way of thinking time-zone shouldn't be changed regularly, let's suppose I as a typical nodejs/express HTTP server developer, have a code like this: (codes are psuedo like code)

app.get(`users`, (req,res) => {
  res.send(sql.get('users', req.params.page));
});

To catch that only one-time time-zone change in system, instead of setting up those system watches I described in paragraph 1, I, as a typical js/nodejs web developer write:

app.get(`users`, (req,res) => {
  require('reset-date-cache`)();
  res.send(sql.get('users', req.params.page));
});

This is the only way I could make sure I catch the correct time zone for each request. which is of course as noted, so expensive and probably shouldn't be called/used like this.

or write a code like:

require('reset-date-cache')();
setInterval(require('reset-date-cache'), _24_HOURS);

This solution, is not complete , as it may miss lots of requests before clearing cache, and in shorter intervals, it still faces performance issues. (and after all, if it is a good solution, why not built-in into nodejs as @jasnell noted) ?

After all I don't think facing problems like we don't think changing time-zones frequently is a good solution for general-purpose programming languages.

Thank you @jasnell for devoting time on this.

@bnoordhuis
Copy link
Member

I can't see much reason why we can't incorporate reset-date-cache into core as part of the v8 module.

Already exists, courtesy of #20026:

process.env.TZ = process.env.TZ || ''  // resets the date cache

@aghArdeshir
Copy link
Author

@bnoordhuis
it does not. If you mean by resetting the date cache I should get the correct time-zone, I don't.

Screenshot from 2020-06-10 06-20-49

@bnoordhuis
Copy link
Member

You misunderstand what reset-date-cache does. It tells V8 to flush its internal date cache, not that it should query the operating system for the current timezone. The latter is Complicated.

If you have some external means of obtaining the current timezone (e.g. timedatectl show -p Timezone on linux), then assigning that to process.env.TZ will reset the date cache.

@aghArdeshir
Copy link
Author

Oh! Yes, that's advanced stuff for me. 😀
Thanks 👍

@jasnell
Copy link
Member

jasnell commented Jun 10, 2020

Thanks @bnoordhuis :-) totally forgot about that change!

@bnoordhuis
Copy link
Member

I don't think there's anything left here to do so I'll take the liberty of closing it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests

3 participants