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

Parsing failures with day of week tokens (ddd or dddd) #4227

Closed
amarzavery opened this Issue Oct 11, 2017 · 5 comments

Comments

Projects
None yet
7 participants
@amarzavery

amarzavery commented Oct 11, 2017

Description of the Issue and Steps to Reproduce:

var timeStringOne = 'Mon, 01 Jan 0001 00:00:00 GMT';
var dateFormat = 'ddd, DD MMM YYYY HH:mm:ss';
moment.utc(timeStringOne, dateFormat).toDate();
  • 2.18.1
fieldDate.toString()
"Sun Dec 31    0 16:00:00 GMT-0800 (Pacific Standard Time)"
  • 2.19.0
fieldDate.toString()
"Invalid Date"

The above snippet works well with 2.18.1 but not with 2.19.0. It gives an InvalidDate.
Please include the values of all variables used.

Environment: Windows 10/ OSX node.js version 7.10 and 8.5.0

Examples: Chrome 49 on OSX, Internet Explorer 10 on Windows 7, Node.JS 4.4.4 on Ubuntu 16.0.4

Both the browser and the OS are important to us, particularly if you have an unsual environment like an IOT application.

Other information that may be helpful:

  • The time zone setting of the machine the code is running on
  • The time and date at which the code was run
  • Other libraries in use (TypeScript, Immutable.js, etc)

If you are reporting an issue, please run the following code in the environment you are using and include the output:

console.log( (new Date()).toString())
console.log((new Date()).toLocaleString())
console.log( (new Date()).getTimezoneOffset())
console.log( navigator.userAgent)
console.log(moment.version)
> var moment = require("moment");
undefined
> console.log( (new Date()).toString())
Tue Oct 10 2017 18:51:04 GMT-0700 (Pacific Daylight Time)
undefined
> console.log((new Date()).toLocaleString())
2017-10-10 18:51:18
undefined
> console.log( (new Date()).getTimezoneOffset())
420
undefined
> console.log( navigator.userAgent)
ReferenceError: navigator is not defined
    at repl:1:14
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at REPLServer.defaultEval (repl.js:239:29)
    at bound (domain.js:301:14)
    at REPLServer.runBound [as eval] (domain.js:314:12)
    at REPLServer.onLine (repl.js:440:10)
    at emitOne (events.js:120:20)
    at REPLServer.emit (events.js:210:7)
    at REPLServer.Interface._onLine (readline.js:279:10)
    at REPLServer.Interface._line (readline.js:626:8)
> console.log(moment.version)
2.19.0
undefined
>

Ensure your issue is isolated to moment. Issues involving third party tools will be closed unless submitted by the tool's author/maintainer.

amarzavery added a commit to amarzavery/autorest.typescript that referenced this issue Oct 11, 2017

amarzavery added a commit to amarzavery/autorest.typescript that referenced this issue Oct 11, 2017

@ZachGawlik

This comment has been minimized.

ZachGawlik commented Oct 11, 2017

I ran into this issue via moment-timezone moment/moment-timezone#545.

I believe it's related to a specific hourly time issue. Potentially tied to inputting the day of week.

const dateFormat = 'ddd, DD MMM YYYY HH:mm:ss';
const test = (timeString) =>moment.utc(timeString, dateFormat).toDate();

test('Mon, 01 Jan 0001 00:00:00 GMT'); // invalid
test('Mon, 01 Jan 0001 04:00:00 GMT'); // invalid
test('Mon, 01 Jan 0001 05:00:00 GMT'); // works

And

> moment.utc('Saturday 04:00', 'dddd HH:mm').format()
'2017-10-14T04:00:00Z'
> moment.utc('Saturday 03:00', 'dddd HH:mm').format()
'Invalid date'
> moment.utc('Saturday 03:59', 'dddd HH:mm').format()
'Invalid date'
> moment.utc('May 03:59', 'MMMM HH:mm').format()
'2017-05-01T03:59:00Z'

Further details:

Wed Oct 11 2017 23:20:01 GMT+0000 (UTC)
CampaignScheduleUtils.coffee?bb14:117 10/11/2017, 7:20:01 PM
CampaignScheduleUtils.coffee?bb14:118 0
CampaignScheduleUtils.coffee?bb14:119 Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36
CampaignScheduleUtils.coffee?bb14:120 2.19.0

@mj1856 mj1856 changed the title from unit tests failing after upgrading to 2.19.0 for minimum date to Parsing failures with day of week tokens (ddd or dddd) Oct 12, 2017

@mj1856

This comment has been minimized.

Member

mj1856 commented Oct 12, 2017

I can confirm that this is indeed happening in 2.19.1. When ddd or dddd tokens are present in the format string, some values are failing to be parsed.

Short term workaround, in the case of ddd, DD MMM YYYY HH:mm:ss, just omit the ddd, portion. Even if the weekday name is in your input string, you don't need to provide it in the format token if you're not in strict parsing mode.

This workaround isn't helpful, of course, if your input is just dddd HH:mm, as one would want the weekday to be considered.

@mj1856 mj1856 added the Bug label Oct 12, 2017

@Mithgol

This comment has been minimized.

Mithgol commented Oct 16, 2017

I am not sure when omitting ddd helps.

For example, 'Invalid date' is returned by both moment('Sun Oct 15 22:41:17 +0000 2017', 'ddd MMM DD HH:mm:ss ZZ YYYY').utc().format('YYYY-MM-DD HH:mm:ss') and moment('Sun Oct 15 22:41:17 +0000 2017', 'MMM DD HH:mm:ss ZZ YYYY').utc().format('YYYY-MM-DD HH:mm:ss') to me, though the latter does not have a ddd.

@vindia

This comment has been minimized.

vindia commented Nov 28, 2017

Just an observation from my side here, when I encountered this issue in my project. Maybe it is helpful.

I'm on a Ubuntu 14.04 VM (through Vagrant) and when I set my local timezone to be UTC I get the following results, using moment version 2.19.2

> moment.tz('maandag 16 oktober 2017, 21:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
true
> moment.tz('maandag 16 oktober 2017, 22:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
true
> moment.tz('dinsdag 16 oktober 2017, 22:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
false // expected, as 16 October was not on a Tuesday

However, when I change my local timezone to CET, using sudo timedatectl set-timezone CET I get the following results:

> moment.tz('maandag 16 oktober 2017, 21:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
true
> moment.tz('maandag 16 oktober 2017, 22:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
false // this is wrong!
> moment.tz('dinsdag 16 oktober 2017, 21:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
true // expected, as 16 October was not on a Tuesday
> moment.tz('dinsdag 16 oktober 2017, 22:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
true // this is wrong!

Could this be related to daylight saving time (CEST)? It changed on October 29th, when I change the date in my examples above to Monday October 30th, I get slightly different (but not error-free) results:

> moment.tz('maandag 30 oktober 2017, 21:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
true
> moment.tz('maandag 30 oktober 2017, 22:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
true
> moment.tz('maandag 16 oktober 2017, 23:22', 'dddd D MMMM YYYY, HH:mm', 'nl', true, 'Europe/Amsterdam').isValid();
false // this is wrong!
@Davy-F

This comment has been minimized.

Davy-F commented Nov 29, 2017

This needs fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment