Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

better error handling for broken database.json files #149

Merged
merged 2 commits into from

3 participants

@acruikshank

I left out a comma in my database.json file the first time I tried to run an update. I got an error like:

Cannot find module '/Path/to/Project/Path/to/project/database.json'

which made me think that the problem was with the location of my config file or with node-db-migrate itself. This pull request throws a syntax error if the file is found in the default location but cannot be read. Thanks.

lib/config.js
@@ -14,6 +14,10 @@ exports.load = function(fileName, currentEnv) {
try {
config = require(fileName);
} catch(e) {
+ // distinguish broken files from missing ones
+ if (e instanceof SyntaxError)
@mex
mex added a note

You might want to add curly brackets to the if-statement to maintain the conventions in the rest of the code.

Good point. I added a commit to fix it.

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

I've stumbled upon this before, it'll be nice to get a syntax error instead of a confusing (and misleading) error message.

@kunklejr kunklejr merged commit f6e8e39 into db-migrate:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 25 additions and 0 deletions.
  1. +5 −0 lib/config.js
  2. +19 −0 test/config_test.js
  3. +1 −0  test/database.notjson
View
5 lib/config.js
@@ -14,6 +14,11 @@ exports.load = function(fileName, currentEnv) {
try {
config = require(fileName);
} catch(e) {
+ // distinguish broken files from missing ones
+ if (e instanceof SyntaxError){
+ throw e;
+ }
+
config = require(path.join(process.cwd(), fileName));
}
View
19 test/config_test.js
@@ -37,6 +37,25 @@ vows.describe('config').addBatch({
assert.equal(current.settings.driver, 'sqlite3');
assert.equal(current.settings.filename, ':memory:');
}
+ },
+}).addBatch({
+ 'loading from a broken config file': {
+ topic: function() {
+ var configPath = path.join(__dirname, 'database.notjson');
+ config.load = _configLoad;
+ config.loadUrl = _configLoadUrl;
+ try {
+ config.load(configPath, 'dev');
+ } catch (e) {
+ return e;
+ }
+ return;
+ },
+
+ 'should throw a syntax error': function (error) {
+ assert.isDefined(error);
+ assert.ok(error instanceof SyntaxError, "Expected broken file to produce syntax error");
+ }
}
}).addBatch({
'loading from a file with ENV vars': {
View
1  test/database.notjson
@@ -0,0 +1 @@
+{this:'is' not:'valid' json}
Something went wrong with that request. Please try again.