Skip to content

Commit 6b096fc

Browse files
committed
new error reporting is finished.
1 parent 0e226d5 commit 6b096fc

File tree

6 files changed

+144
-65
lines changed

6 files changed

+144
-65
lines changed

lib/events.js

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,11 @@ var $events = {
5656
if (ctx.options.connect instanceof Function) {
5757
try {
5858
ctx.options.connect(client, ctx.dc, isFresh);
59-
} catch (err) {
59+
} catch (e) {
6060
// have to silence errors here;
6161
// cannot allow unhandled errors while connecting to the database,
6262
// as it will break the connection logic;
63-
$events.unexpected('connect', err);
63+
$events.unexpected('connect', e);
6464
}
6565
}
6666
},
@@ -97,11 +97,11 @@ var $events = {
9797
if (ctx.options.disconnect instanceof Function) {
9898
try {
9999
ctx.options.disconnect(client, ctx.dc);
100-
} catch (err) {
100+
} catch (e) {
101101
// have to silence errors here;
102102
// cannot allow unhandled errors while disconnecting from the database,
103103
// as it will break the disconnection logic;
104-
$events.unexpected('disconnect', err);
104+
$events.unexpected('disconnect', e);
105105
}
106106
}
107107
},
@@ -168,10 +168,10 @@ var $events = {
168168
if (options.query instanceof Function) {
169169
try {
170170
options.query(context);
171-
} catch (err) {
171+
} catch (e) {
172172
// throwing an error during event 'query'
173173
// will result in a reject for the request.
174-
return new $npm.utils.InternalError(err);
174+
return e instanceof Error ? e : new $npm.utils.InternalError(e);
175175
}
176176
}
177177
},
@@ -235,10 +235,10 @@ var $events = {
235235
if (options.receive instanceof Function) {
236236
try {
237237
options.receive(data, result, context);
238-
} catch (err) {
238+
} catch (e) {
239239
// throwing an error during event 'receive'
240240
// will result in a reject for the request.
241-
return new $npm.utils.InternalError(err);
241+
return e instanceof Error ? e : new $npm.utils.InternalError(e);
242242
}
243243
}
244244
},
@@ -279,9 +279,9 @@ var $events = {
279279
if (options.task instanceof Function) {
280280
try {
281281
options.task(context);
282-
} catch (err) {
282+
} catch (e) {
283283
// silencing the error, to avoid breaking the task;
284-
$events.unexpected('task', err);
284+
$events.unexpected('task', e);
285285
}
286286
}
287287
},
@@ -322,9 +322,9 @@ var $events = {
322322
if (options.transact instanceof Function) {
323323
try {
324324
options.transact(context);
325-
} catch (err) {
325+
} catch (e) {
326326
// silencing the error, to avoid breaking the transaction;
327-
$events.unexpected('transact', err);
327+
$events.unexpected('transact', e);
328328
}
329329
}
330330
},
@@ -379,11 +379,11 @@ var $events = {
379379
if (options.error instanceof Function) {
380380
try {
381381
options.error(err, context);
382-
} catch (err) {
382+
} catch (e) {
383383
// have to silence errors here;
384384
// throwing unhandled errors while handling an error
385385
// notification is simply not acceptable.
386-
$events.unexpected('error', err);
386+
$events.unexpected('error', e);
387387
}
388388
}
389389
},
@@ -468,22 +468,22 @@ var $events = {
468468
if (options.extend instanceof Function) {
469469
try {
470470
options.extend.call(obj, obj, dc);
471-
} catch (err) {
471+
} catch (e) {
472472
// have to silence errors here;
473473
// the result of throwing unhandled errors while
474474
// extending the protocol would be unpredictable.
475-
$events.unexpected('extend', err);
475+
$events.unexpected('extend', e);
476476
}
477477
}
478478
},
479479

480480
/**
481481
* @event unexpected
482482
* @param {string} event - unhandled event name.
483-
* @param {String|Error} err - unhandled error.
483+
* @param {String|Error} e - unhandled error.
484484
* @private
485485
*/
486-
unexpected: function (event, err) {
486+
unexpected: function (event, e) {
487487
// If you should ever get here, your app is definitely broken, and you need to fix
488488
// your event handler to prevent unhandled errors during event notifications.
489489
//
@@ -492,11 +492,8 @@ var $events = {
492492

493493
/* istanbul ignore if */
494494
if (!$npm.main.suppressErrors) {
495-
$npm.con.error("Unexpected error in '%s' event handler.", event);
496-
if (!$npm.utils.isNull(err)) {
497-
$npm.con.error(err.stack || err.message || err);
498-
}
499-
console.log('\n');
495+
var stack = e instanceof Error ? e.stack : new Error().stack;
496+
$npm.con.error("Unexpected error in '%s' event handler.\n%s\n", event, stack);
500497
}
501498
}
502499
};

lib/query.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ var $npm = {
1313
};
1414

1515
var QueryResultError = $npm.errors.QueryResultError,
16+
InternalError = $npm.utils.InternalError,
1617
ExternalQuery = $npm.types.ExternalQuery,
1718
PreparedStatement = $npm.types.PreparedStatement,
1819
ParameterizedQuery = $npm.types.ParameterizedQuery,
19-
InternalError = $npm.utils.InternalError,
2020
SpecialQuery = $npm.special.SpecialQuery,
2121
qrec = $npm.errors.queryResultErrorCode;
2222

@@ -112,7 +112,7 @@ function $query(ctx, query, values, qrm, config) {
112112
var prefix = capSQL ? 'SELECT * FROM' : 'select * from';
113113
query = prefix + ' ' + query + '(...)';
114114
}
115-
error = e;
115+
error = e instanceof Error ? e : new $npm.utils.InternalError(e);
116116
params = values;
117117
}
118118
}
@@ -203,10 +203,9 @@ function $query(ctx, query, values, qrm, config) {
203203

204204
function notifyReject() {
205205
var context = getContext();
206-
if (error !== undefined) {
207-
// TODO: Consider using a custom error type here instead:
206+
if (error) {
208207
if (error instanceof InternalError) {
209-
error = new Error(error.message);
208+
error = error.error;
210209
}
211210
$npm.events.error(opt, error, context);
212211
reject(error);

lib/stream.js

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,10 @@ function $stream(ctx, qs, initCB, config) {
2929
// parameter `initCB` must be passed as the initialization callback;
3030
return $p.reject(new TypeError("Invalid or missing stream initialization callback."));
3131
}
32-
var errorMsg = $npm.events.query(ctx.options, getContext());
33-
if (errorMsg) {
34-
errorMsg = errorMsg.message;
35-
$npm.events.error(ctx.options, errorMsg, getContext());
36-
return $p.reject(errorMsg);
32+
var error = $npm.events.query(ctx.options, getContext());
33+
if (error) {
34+
$npm.events.error(ctx.options, error, getContext());
35+
return $p.reject(error);
3736
}
3837
var stream, fetch, start, nRows = 0;
3938
try {
@@ -44,10 +43,10 @@ function $stream(ctx, qs, initCB, config) {
4443
if (!err && rows.length) {
4544
nRows += rows.length;
4645
var context = getContext();
47-
if (errorMsg === undefined) {
48-
errorMsg = $npm.events.receive(ctx.options, rows, undefined, context);
46+
if (error === undefined) {
47+
error = $npm.events.receive(ctx.options, rows, undefined, context);
4948
}
50-
if (errorMsg !== undefined) {
49+
if (error !== undefined) {
5150
stream.close();
5251
}
5352
}
@@ -57,19 +56,19 @@ function $stream(ctx, qs, initCB, config) {
5756
start = Date.now();
5857
initCB.call(this, stream); // the stream must be initialized during the call;
5958
} catch (err) {
60-
errorMsg = err;
59+
error = err;
6160
}
62-
if (errorMsg) {
61+
if (error) {
6362
// error thrown by initCB();
6463
stream._fetch = fetch;
65-
$npm.events.error(ctx.options, errorMsg, getContext());
66-
return $p.reject(errorMsg);
64+
$npm.events.error(ctx.options, error, getContext());
65+
return $p.reject(error);
6766
}
6867
return $p(function (resolve, reject) {
6968
stream.once('end', function () {
7069
stream._fetch = fetch;
71-
if (errorMsg) {
72-
onError(errorMsg);
70+
if (error) {
71+
onError(error);
7372
} else {
7473
resolve({
7574
processed: nRows, // total number of rows processed;
@@ -83,7 +82,7 @@ function $stream(ctx, qs, initCB, config) {
8382
});
8483
function onError(e) {
8584
if (e instanceof $npm.utils.InternalError) {
86-
e = e.message;
85+
e = e.error;
8786
}
8887
$npm.events.error(ctx.options, e, getContext());
8988
reject(e);
@@ -95,7 +94,7 @@ function $stream(ctx, qs, initCB, config) {
9594
if (ctx.db) {
9695
client = ctx.db.client;
9796
} else {
98-
errorMsg = "Loose request outside an expired connection.";
97+
error = new Error("Loose request outside an expired connection.");
9998
}
10099
return {
101100
client: client,

lib/utils/index.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,6 @@ function getSafeConnection(cn) {
8787
});
8888
}
8989

90-
//////////////////////////////
91-
// Internal error container;
92-
function InternalError(message) {
93-
this.message = message;
94-
}
95-
9690
///////////////////////////////////////////
9791
// Returns a space gap for console output;
9892
function messageGap(level) {
@@ -133,7 +127,14 @@ function getLocalStack(startIdx) {
133127
}).join('\n');
134128
}
135129

130+
//////////////////////////////
131+
// Internal error container;
132+
function InternalError(error) {
133+
this.error = error;
134+
}
135+
136136
var exp = {
137+
InternalError: InternalError,
137138
getLocalStack: getLocalStack,
138139
isPathAbsolute: isPathAbsolute,
139140
lock: lock,
@@ -143,7 +144,6 @@ var exp = {
143144
addReadProp: addReadProp,
144145
addReadProperties: addReadProperties,
145146
getSafeConnection: getSafeConnection,
146-
InternalError: InternalError,
147147
messageGap: messageGap,
148148
inherits: inherits
149149
};

test/dbSpec.js

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1681,6 +1681,91 @@ describe("Task", function () {
16811681

16821682
});
16831683

1684+
describe("negative query formatting", function () {
1685+
1686+
describe("with invalid property name", function () {
1687+
var error;
1688+
beforeEach(function (done) {
1689+
db.one('select ${invalid}', {})
1690+
.catch(function (e) {
1691+
error = e;
1692+
done();
1693+
});
1694+
});
1695+
it("must reject with correct error", function () {
1696+
expect(error instanceof Error).toBe(true);
1697+
expect(error.message).toBe("Property 'invalid' doesn't exist.");
1698+
});
1699+
});
1700+
1701+
describe("with invalid variable index", function () {
1702+
var error;
1703+
beforeEach(function (done) {
1704+
db.one('select $1', [])
1705+
.catch(function (e) {
1706+
error = e;
1707+
done();
1708+
});
1709+
});
1710+
it("must reject with correct error", function () {
1711+
expect(error instanceof RangeError).toBe(true);
1712+
expect(error.message).toBe("Variable $1 out of range. Parameters array length: 0");
1713+
});
1714+
});
1715+
1716+
describe("with formatting parameter throwing error", function () {
1717+
var error;
1718+
beforeEach(function (done) {
1719+
db.one('select $1', [function () {
1720+
throw new Error("ops!");
1721+
}])
1722+
.catch(function (e) {
1723+
error = e;
1724+
done();
1725+
});
1726+
});
1727+
it("must reject with correct error", function () {
1728+
expect(error instanceof Error).toBe(true);
1729+
expect(error.message).toBe("ops!");
1730+
});
1731+
});
1732+
1733+
describe("with formatting parameter throwing value", function () {
1734+
var error;
1735+
beforeEach(function (done) {
1736+
db.one('select $1', [function () {
1737+
throw 123;
1738+
}])
1739+
.catch(function (e) {
1740+
error = e;
1741+
done();
1742+
});
1743+
});
1744+
it("must reject with correct error", function () {
1745+
expect(error).toBe(123);
1746+
});
1747+
});
1748+
1749+
describe("with formatting parameter throwing undefined", function () {
1750+
var error, handled;
1751+
beforeEach(function (done) {
1752+
db.one('select $1', [function () {
1753+
throw undefined;
1754+
}])
1755+
.catch(function (e) {
1756+
handled = true;
1757+
error = e;
1758+
done();
1759+
});
1760+
});
1761+
it("must reject with correct error", function () {
1762+
expect(handled).toBe(true);
1763+
expect(error).toBeUndefined();
1764+
});
1765+
});
1766+
1767+
});
1768+
16841769
if (jasmine.Runner) {
16851770
var _finishCallback = jasmine.Runner.prototype.finishCallback;
16861771
jasmine.Runner.prototype.finishCallback = function () {

0 commit comments

Comments
 (0)