Skip to content

Commit

Permalink
workaround for Array builtin call on proxied array
Browse files Browse the repository at this point in the history
Calling the builtin function 'sort' on an array wrapped in an ES6
proxy results in "illegal access" being thrown (a string, not an
Error). This is probably related in some way to these issues:
tvcutsem/harmony-reflect#6
tvcutsem/harmony-reflect#19

This change fixes the problem for the time being through a rather
explicit workaround; if the issue also occurs for other functions, it
will likely surface again.
(also, the change in Session.js makes sure error log entry is nicely
formatted even when strings are thrown)
  • Loading branch information
yelworc committed Nov 15, 2014
1 parent a94f831 commit d92aa3b
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/comm/Session.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,14 @@ Session.prototype.postRequestProc = function postRequestProc(req) {
Session.prototype.handleAmfReqError = function handleAmfReqError(err, req) {
if (typeof err === 'object' && err.type === 'stack_overflow') {
// special treatment for stack overflow errors
// see <https://github.com/trentm/node-bunyan/issues/127>
// see https://github.com/trentm/node-bunyan/issues/127
err = new Error(err.message);
}
if (typeof err === 'string') {
// catch malcontents throwing strings instead of Errors, e.g.
// https://github.com/tvcutsem/harmony-reflect/issues/38
err = new Error(err);
}
log.error(err, 'error processing %s request for %s', req.type, this.pc);
if (this.pc && req.id) {
// send error response back to client
Expand Down
6 changes: 6 additions & 0 deletions src/data/persProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ function makeProxy(obj, prop) {
if (name === '__isPP') {
return true;
}
if (name === 'sort' && typeof target.sort === 'function' &&
target instanceof Array) {
// workaround for builtin Array.prototype.sort (calling it on a
// proxied array throws "illegal access" string otherwise)
return target.sort.bind(target);
}
if (name === 'valueOf' || name === 'toString') {
return function () {
return '^P' + target.toString();
Expand Down
9 changes: 9 additions & 0 deletions test/unit/data/persProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,14 @@ suite('persProxy', function () {
assert.strictEqual(JSON.stringify(p),
'{"tsid":"x","arr":[{"x":3},[1,2,3]]}');
});

test('pproxy does not break Array.sort', function () {
var arr = ['alph', 'humbaba', 'cosma', 'spriggan'];
var p = pp.makeProxy(arr);
var sorted = p.sort(function (a, b) {
return a < b;
});
assert.deepEqual(sorted, ['spriggan', 'humbaba', 'cosma', 'alph']);
});
});
});

0 comments on commit d92aa3b

Please sign in to comment.