sidorares / node-mysql2

:zap: fast mysqljs/mysql compatible mysql driver for node.js
https://sidorares.github.io/node-mysql2/
MIT License
4.04k stars 612 forks source link

Remove error being logged for invalid options AND please do not make it ever throw an error for this! #2481

Open dhurtrci opened 6 months ago

dhurtrci commented 6 months ago

I would like the console logging in the code below (in connection_config.js) to be removed. Also I would ask that this is never made into a thrown error (as is threatened).

if (validOptions[key] !== 1) {
        // REVIEW: Should this be emitted somehow?
        // eslint-disable-next-line no-console
        console.error(
          `Ignoring invalid configuration option passed to Connection: ${key}. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection`
        );
      }

My reasons are that some other libraries (e.g. loopback-connector-mysql) add 'harmless' extra (non MySql/MariaDB) properties to the object passed here (for other purposes) which leads to this error being logged (and the extra properties do not actually cause problems).

Additionally, having this check (and - more alarmingly - threatening to throw an error in future!) means that the code in mysql2 has to be sure to always keep an up-to-date list of options. Additionally it neglects that different versions of MySQL/MariaDB might in fact have a slightly different list of valid options.

My opinion is that the logging (and check of property name validity) should be removed.

wellwelwel commented 6 months ago

How about a simple option like noWarnings or hideWarnings at the connection level?

That could solve this issue and the #2471.

Do you think that's a bad idea/practice?

Thallius commented 3 months ago

Can someone explain this code part to me? (text_parser.js line 91)

string: function (encoding = field.encoding) { if (field.columnType === Types.JSON && encoding === field.encoding) {

For me this If must always be true if field.columnType is a JSON or?