Mailtrain-org / mailtrain

Self hosted newsletter app
GNU General Public License v3.0
5.52k stars 692 forks source link

TypeError: entry.*.toISOString is not a function #311

Closed bbbart closed 7 years ago

bbbart commented 7 years ago

I encountered a problem with importing a CSV file of list subscribers into a fresh install (latest git commit: 6aec53d88285f1f0e60ef4957a3498912051322c).

TypeError: entry.created.toISOString is not a function (and later the same with entry.finished.toISOString too). This happes in routes/lists.js on lines 306 and 307.

I have never worked in nodejs, so I simply changed those lines to entry.created = entry.created; // && entry.created.toISOString(); and entry.finished = entry.finished; // && entry.finished.toISOString();. This resulted in a seemingly successful import, except that the Created column of the subscribers is 'N/A' everywhere.

I'd be happy to test more if it can help to resolve this issue.

bbbart commented 7 years ago

After playing around little more, I got a similar error after editing an existing template with GrapeJS: TypeError: row.created.toISOString is not a function from /var/www/mailtrain/routes/campaigns.js:275:70, again resulting in a crash of mailtrain.

witzig commented 7 years ago

When you create a template, is the timestamp correctly set in the database? See here.

bbbart commented 7 years ago

Yup!

MariaDB [MAILTRAIN]> select id, name, description, editor_name, created from templates;
+----+---------------+----------------------------------------+-------------+---------------------+
| id | name          | description                            | editor_name | created             |
+----+---------------+----------------------------------------+-------------+---------------------+
|  1 | Test Template | Just a template for testing out stuff. | grapejs     | 2017-09-15 12:06:57 |
+----+---------------+----------------------------------------+-------------+---------------------+
1 row in set (0.00 sec)

The error only came after editing the template, not after initially creating it.

Also:

MariaDB [MAILTRAIN]> select id, list, type, created, finished from importer;
+----+------+------+---------------------+---------------------+
| id | list | type | created             | finished            |
+----+------+------+---------------------+---------------------+
|  1 |    1 |    1 | 2017-09-15 11:27:26 | NULL                |
|  2 |    1 |    1 | 2017-09-15 11:27:49 | 2017-09-15 11:42:10 |
+----+------+------+---------------------+---------------------+
2 rows in set (0.00 sec)

and:

MariaDB [MAILTRAIN]> select id, tz, imported, status, is_test, status_change, created from subscription__1;
+----+-----------------+----------+--------+---------+---------------------+---------------------+
| id | tz              | imported | status | is_test | status_change       | created             |
+----+-----------------+----------+--------+---------+---------------------+---------------------+
|  1 | europe/brussels |        2 |      1 |       1 | 2017-09-15 09:42:10 | 2017-09-15 11:42:10 |
|  2 | europe/brussels |        2 |      1 |       0 | 2017-09-15 09:42:10 | 2017-09-15 11:42:10 |
|  3 | asia/karachi    |        2 |      1 |       0 | 2017-09-15 09:42:10 | 2017-09-15 11:42:10 |
|  4 | asia/colombo    |        2 |      1 |       0 | 2017-09-15 09:42:10 | 2017-09-15 11:42:10 |
|  5 | europe/brussels |        2 |      1 |       0 | 2017-09-15 09:42:10 | 2017-09-15 11:42:10 |
|  6 | asia/karachi    |        2 |      1 |       0 | 2017-09-15 09:42:10 | 2017-09-15 11:42:10 |
|  7 | utc             |     NULL |      1 |       0 | 2017-09-15 10:24:54 | 2017-09-15 12:24:54 |
+----+-----------------+----------+--------+---------+---------------------+---------------------+
7 rows in set (0.00 sec)

The first import never finished, because I didn't let it happen (my CSV file didn't have a header line, so the mapping was messed up).

witzig commented 7 years ago

Thanks. Might be the mysql package is not returning date types as Date objects in your case?

In the root directory of your Mailtrain installation, could you execute NODE_ENV=production node and then paste this snippet:

const db = require('./lib/db');

db.getConnection((err, connection) => {
    connection.query('select created from templates', (err, rows) => {
        console.log('instanceof Date:', rows[0].created instanceof Date);
    });
});

It should output instanceof Date: true. Yes?

bbbart commented 7 years ago

No, it doesn't: instanceof Date: false. However, I'm not sure if the test worked correctly, since the variable db is undefined. ./lib/db.js does exist however. I tried it both with mailtrain still running (I'm using systemd for this) and after having stopped it.

witzig commented 7 years ago

If it didn't throw an error then it worked correctly. If you want to verify, try:

const db = require('./lib/db');

db.getConnection((err, connection) => {
    connection.query('select created from templates', (err, rows) => {
        console.log('created:', rows[0].created);
        console.log('created is typeof:', typeof rows[0].created);
        console.log('created is instanceof Date:', rows[0].created instanceof Date);
    });
});

It should output:

created: 2017-04-04T14:39:18.000Z
created is typeof: object
created is instanceof Date: true

Looks like we found the problem, but how to fix it ... ? The dateStrings option is false by default and you surely didn't change this.

Is MariaDB configured correctly? Which version of Node are you using? Are your Packages up to date?

bbbart commented 7 years ago

Aah yes, you're right. I guess I'm too used to the Python REPL, so I'm misunderstanding node.

Your snippet above gives me:

created: 2017-09-15 12:06:57
created is typeof: string
created is instanceof Date: false

There a clear difference in the formatting of the created value. Hm...

MariaDB is configured quite standardly, just through a simple apt-get install on Debian 7.9 (mariadb-server 10.0.32-9+deb8u1). I'm using node v8.5.0 (not installed through apt, since that gave me the ancient version 0.10 or something). The .deb packages are all up to date (did an apt-get update/upgrade/autoremove only yesterday) and all node packages are installed also only yesterday as part of the Mailtrain installation procedure (no other node packages are installed on the system).

And as for the DB structure, I have:

MariaDB [MAILTRAIN]> describe templates;
+-------------+------------------+------+-----+-------------------+----------------+
| Field       | Type             | Null | Key | Default           | Extra          |
+-------------+------------------+------+-----+-------------------+----------------+
| id          | int(11) unsigned | NO   | PRI | NULL              | auto_increment |
| name        | varchar(255)     | NO   | MUL |                   |                |
| description | text             | YES  |     | NULL              |                |
| editor_name | varchar(50)      | YES  |     |                   |                |
| editor_data | longtext         | YES  |     | NULL              |                |
| html        | longtext         | YES  |     | NULL              |                |
| text        | longtext         | YES  |     | NULL              |                |
| created     | timestamp        | NO   |     | CURRENT_TIMESTAMP |                |
+-------------+------------------+------+-----+-------------------+----------------+
8 rows in set (0.00 sec)

which looks correct, right?

Finally, npm tells me I'm using package mysql@2.14.1. I indeed didn't mess around with the dateStrings setting, but rows[0].created.constructor does indeed give me [Function: String].

witzig commented 7 years ago

Thanks, looks good to me.

I'm running out of ideas, yet the next thing I'd try is to insert some debug statements in the mysql package, see ./node_modules/mysql/lib/protocol/packets/RowDataPacket.js, L57-87. Is the field.type detected correctly? If so, surely it fails here?

I'm afraid I'm not a big help here ...

bbbart commented 7 years ago

I think you're a great help, thank you so much. Also, I think you are right: whenever I manually add a contact to a list, it detects the field.type correctly as a datefield, but then invariably goes inside the NaN-block.

I also think I found the problem: new Date('2017-09-15 14:20:29') works just fine, but new Date('2017-09-15 14:20:29 Europe/Brussels') doesn't. The "Europe/Brussels" gets added because I have timezone="Europe/Brussels" in the [mysql]-block of production.toml.

I changed that to 'local' and everything seems to work just fine now. Maybe there is some note to be made about this in the documentation or installation instructions.

Thank you so much. Great help, really.

witzig commented 7 years ago

Ah, yes. That explains it all. As per the mysql readme:

timezone: The timezone configured on the MySQL server. This is used to type cast server date/time values to JavaScript Date object and vice versa. This can be 'local', 'Z', or an offset in the form +HH:MM or -HH:MM. (Default: 'local')

Glad you got the mystery solved. I will add a note to the config file.