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.

Sign up for GitHub

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

Remove linebreaks from long error messages #1606

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove linebreaks from long error messages #1606

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 84 additions & 88 deletions lib/waterline/utils/query/process-all-records.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,23 @@ var eachRecordDeep = require('waterline-utils').eachRecordDeep;
var WARNING_SUFFIXES = {

MIGHT_BE_YOUR_FAULT:
'\n'+
'> You are seeing this warning because there are records in your database that don\'t\n'+
'> match up with your models. This is often the result of a model definition being\n'+
'> changed without also migrating leftover data. But it could also be because records\n'+
'> were added or modified in your database from somewhere outside of Sails/Waterline\n'+
'> (e.g. phpmyadmin, or another app). In either case, to make this warning go away,\n'+
'> you have a few options. First of all, you could change your model definition so\n'+
'> that it matches the existing records in your database. Or you could update/destroy\n'+
'> the old records in your database; either by hand, or using a migration script.\n'+
'> \n'+
(process.env.NODE_ENV !== 'production' ? '> (For example, to wipe all data, you might just use `migrate: drop`.)\n' : '')+
'> \n'+
'> More rarely, this warning could mean there is a bug in the adapter itself. If you\n'+
'> believe that is the case, then please contact the maintainer of this adapter by opening\n'+
'> an issue, or visit http://sailsjs.com/support for help.\n',
'You are seeing this warning because there are records in your database that don\'t '+
'match up with your models. This is often the result of a model definition being '+
'changed without also migrating leftover data. But it could also be because records '+
'were added or modified in your database from somewhere outside of Sails/Waterline '+
'(e.g. phpmyadmin, or another app). In either case, to make this warning go away, '+
'you have a few options. First of all, you could change your model definition so '+
'that it matches the existing records in your database. Or you could update/destroy '+
'the old records in your database; either by hand, or using a migration script. '+
(process.env.NODE_ENV !== 'production' ? '(For example, to wipe all data, you might just use `migrate: drop`.) ' : '')+
'More rarely, this warning could mean there is a bug in the adapter itself. If you '+
'believe that is the case, then please contact the maintainer of this adapter by opening '+
'an issue, or visit http://sailsjs.com/support for help. ',

HARD_TO_SEE_HOW_THIS_COULD_BE_YOUR_FAULT:
'\n'+
'> This is usally caused by a bug in the adapter itself. If you believe that\n'+
'> might be the case here, then please contact the maintainer of this adapter by\n'+
'> opening an issue, or visit http://sailsjs.com/support for help.\n'
'This is usally caused by a bug in the adapter itself. If you believe that '+
'might be the case here, then please contact the maintainer of this adapter by '+
'opening an issue, or visit http://sailsjs.com/support for help. '

};

Expand Down Expand Up @@ -147,17 +143,17 @@ module.exports = function processAllRecords(records, meta, modelIdentity, orm) {
// So if we made it here, we can safely assume that this is due
// to an issue in the _adapter_ -- not some problem with unmigrated
// data.
console.warn('\n'+
'Warning: A record in this result set has extraneous properties ('+nonAttrKeys+')\n'+
'that, after adjusting for any custom columnNames, still do not correspond\n'+
'any recognized attributes of this model (`'+WLModel.identity+'`).\n'+
'Since this model is defined as `schema: true`, this behavior is unexpected.\n'+
console.warn(
'Warning: A record in this result set has extraneous properties ('+nonAttrKeys+') '+
'that, after adjusting for any custom columnNames, still do not correspond '+
'any recognized attributes of this model (`'+WLModel.identity+'`). '+
'Since this model is defined as `schema: true`, this behavior is unexpected. '+
// ====================================================================================
// Removed this for the sake of brevity-- could bring it back if deemed helpful.
// ====================================================================================
// 'This problem could be the result of an adapter method not properly observing\n'+
// 'the `select` clause it receives in the incoming criteria (or otherwise sending\n'+
// 'extra, unexpected properties on records that were left over from old data).\n'+
// 'This problem could be the result of an adapter method not properly observing '+
// 'the `select` clause it receives in the incoming criteria (or otherwise sending '+
// 'extra, unexpected properties on records that were left over from old data). '+
// ====================================================================================
WARNING_SUFFIXES.MIGHT_BE_YOUR_FAULT
);
Expand Down Expand Up @@ -189,12 +185,12 @@ module.exports = function processAllRecords(records, meta, modelIdentity, orm) {
// (but if it was, log a warning. Note that we don't strip it out like
// we would normally, because we're careful not to munge data in this utility.)
if(_.isUndefined(record[key])){
console.warn('\n'+
'Warning: A database adapter should never send back records that have `undefined`\n'+
'on the RHS of any property (e.g. `foo: undefined`). But after transforming\n'+
'columnNames back to attribute names for the model `' + modelIdentity + '`, one\n'+
'of the records sent back from this adapter has a property (`'+key+'`) with\n'+
'`undefined` on the right-hand side.\n' +
console.warn(
'Warning: A database adapter should never send back records that have `undefined` '+
'on the RHS of any property (e.g. `foo: undefined`). But after transforming '+
'columnNames back to attribute names for the model `' + modelIdentity + '`, one '+
'of the records sent back from this adapter has a property (`'+key+'`) with '+
'`undefined` on the right-hand side. ' +
WARNING_SUFFIXES.HARD_TO_SEE_HOW_THIS_COULD_BE_YOUR_FAULT
);
}//>-
Expand Down Expand Up @@ -228,15 +224,15 @@ module.exports = function processAllRecords(records, meta, modelIdentity, orm) {
);

if (!isProbablyValidPkValue) {
console.warn('\n'+
'Warning: Records sent back from a database adapter should always have a valid property\n'+
'that corresponds with the primary key attribute (`'+WLModel.primaryKey+'`). But in this result set,\n'+
'after transforming columnNames back to attribute names for model `' + modelIdentity + '`,\n'+
'there is a record with a missing or invalid `'+WLModel.primaryKey+'`.\n'+
'Record:\n'+
'```\n'+
util.inspect(record, {depth:5})+'\n'+
'```\n'+
console.warn(
'Warning: Records sent back from a database adapter should always have a valid property '+
'that corresponds with the primary key attribute (`'+WLModel.primaryKey+'`). But in this result set, '+
'after transforming columnNames back to attribute names for model `' + modelIdentity + '`, '+
'there is a record with a missing or invalid `'+WLModel.primaryKey+'`. '+
'Record: '+
'``` '+
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alxndrsn util.inspect is going to add new lines here anyway, so I think in spots like this it may just make more sense to leave the lines before and after the backticks. Any thoughts on this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ in which case I should probably just override console.log, console.error etc. 😕

util.inspect(record, {depth:5})+' '+
'``` '+
WARNING_SUFFIXES.MIGHT_BE_YOUR_FAULT
);
}
Expand Down Expand Up @@ -277,18 +273,18 @@ module.exports = function processAllRecords(records, meta, modelIdentity, orm) {
}
// Otherwise, the value is invalid.
else {
console.warn('\n'+
'An association in a result record has an unexpected data type. Since `'+attrName+'` is\n'+
'a singular (association), it should come back from Waterline as either:\n'+
'• `null` (if not populated and set to null explicitly, or populated but orphaned)\n'+
'• a dictionary (if successfully populated), or\n'+
'• a valid primary key value for the associated model (if set + not populated)\n'+
'But for this record, after converting column names back into attribute names, it\n'+
'wasn\'t any of those things.\n'+
'Record:\n'+
'```\n'+
util.inspect(record, {depth:5})+'\n'+
'```\n'+
console.warn(
'An association in a result record has an unexpected data type. Since `'+attrName+'` is '+
'a singular (association), it should come back from Waterline as either: '+
'• `null` (if not populated and set to null explicitly, or populated but orphaned) '+
'• a dictionary (if successfully populated), or '+
'• a valid primary key value for the associated model (if set + not populated) '+
'But for this record, after converting column names back into attribute names, it '+
'wasn\'t any of those things. '+
'Record: '+
'``` '+
util.inspect(record, {depth:5})+' '+
'``` '+
WARNING_SUFFIXES.MIGHT_BE_YOUR_FAULT
);
}
Expand Down Expand Up @@ -318,19 +314,19 @@ module.exports = function processAllRecords(records, meta, modelIdentity, orm) {
}
// Otherwise, the value is invalid.
else {
console.warn('\n'+
'An association in a result record has an unexpected data type. Since `'+attrName+'` is\n'+
'a plural (association), it should come back from Waterline as either:\n'+
'• `undefined` (if not populated), or\n'+
'• an array of child records (if populated)\n'+
'But for this record, it wasn\'t any of those things.\n'+
console.warn(
'An association in a result record has an unexpected data type. Since `'+attrName+'` is '+
'a plural (association), it should come back from Waterline as either: '+
'• `undefined` (if not populated), or '+
'• an array of child records (if populated) '+
'But for this record, it wasn\'t any of those things. '+
// Note that this could mean there was something else already there
// (imagine changing your model to use a plural association instead
// of an embedded array from a `type: 'json'` attribute)
'Record:\n'+
'```\n'+
util.inspect(record, {depth:5})+'\n'+
'```\n'+
'Record: '+
'``` '+
util.inspect(record, {depth:5})+' '+
'``` '+
WARNING_SUFFIXES.MIGHT_BE_YOUR_FAULT
);
}
Expand Down Expand Up @@ -370,15 +366,15 @@ module.exports = function processAllRecords(records, meta, modelIdentity, orm) {
);

if (!isProbablyValidTimestamp) {
console.warn('\n'+
'Warning: After transforming columnNames back to attribute names for model `' + modelIdentity + '`,\n'+
' a record in the result has a value with an unexpected data type for property `'+attrName+'`.\n'+
'The model\'s `'+attrName+'` attribute declares itself an auto timestamp with\n'+
'`type: \''+attrDef.type+'\'`, but instead of a valid timestamp, the actual value\n'+
'in the record is:\n'+
'```\n'+
util.inspect(record[attrName],{depth:5})+'\n'+
'```\n'+
console.warn(
'Warning: After transforming columnNames back to attribute names for model `' + modelIdentity + '`, '+
' a record in the result has a value with an unexpected data type for property `'+attrName+'`. '+
'The model\'s `'+attrName+'` attribute declares itself an auto timestamp with '+
'`type: \''+attrDef.type+'\'`, but instead of a valid timestamp, the actual value '+
'in the record is: '+
'``` '+
util.inspect(record[attrName],{depth:5})+' '+
'``` '+
WARNING_SUFFIXES.MIGHT_BE_YOUR_FAULT
);
}
Expand Down Expand Up @@ -426,25 +422,25 @@ module.exports = function processAllRecords(records, meta, modelIdentity, orm) {
case 'E_INVALID':

if (_.isNull(record[attrName])) {
console.warn('\n'+
'Warning: After transforming columnNames back to attribute names for model `' + modelIdentity + '`,\n'+
' a record in the result has a value of `null` for property `'+attrName+'`.\n'+
'Since the `'+attrName+'` attribute declares `type: \''+attrDef.type+'\'`,\n'+
'without ALSO declaring `allowNull: true`, this `null` value is unexpected.\n'+
'(To resolve, either change this attribute to `allowNull: true` or update\n'+
'existing records in the database accordingly.)\n'+
console.warn(
'Warning: After transforming columnNames back to attribute names for model `' + modelIdentity + '`, '+
' a record in the result has a value of `null` for property `'+attrName+'`. '+
'Since the `'+attrName+'` attribute declares `type: \''+attrDef.type+'\'`, '+
'without ALSO declaring `allowNull: true`, this `null` value is unexpected. '+
'(To resolve, either change this attribute to `allowNull: true` or update '+
'existing records in the database accordingly.) '+
WARNING_SUFFIXES.MIGHT_BE_YOUR_FAULT
);
}
else {
console.warn('\n'+
'Warning: After transforming columnNames back to attribute names for model `' + modelIdentity + '`,\n'+
' a record in the result has a value with an unexpected data type for property `'+attrName+'`.\n'+
'The corresponding attribute declares `type: \''+attrDef.type+'\'` but instead\n'+
'of that, the actual value is:\n'+
'```\n'+
util.inspect(record[attrName],{depth:5})+'\n'+
'```\n'+
console.warn(
'Warning: After transforming columnNames back to attribute names for model `' + modelIdentity + '`, '+
' a record in the result has a value with an unexpected data type for property `'+attrName+'`. '+
'The corresponding attribute declares `type: \''+attrDef.type+'\'` but instead '+
'of that, the actual value is: '+
'``` '+
util.inspect(record[attrName],{depth:5})+' '+
'``` '+
WARNING_SUFFIXES.MIGHT_BE_YOUR_FAULT
);
}
Expand Down