worker: improve error (de)serialization

Rather than passing errors using some sort of string representation, do a best effort for faithful serialization/deserialization of uncaught exception objects. PR-URL: #20876 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Shingo Inoue <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Gus Caplan <[email protected]>

Author: addaleax

lib/internal/error-serdes.js

@@ -0,0 +1,121 @@
+'use strict';
+
+constBuffer=require('buffer').Buffer;
+const{ serialize, deserialize }=require('v8');
+const{ SafeSet }=require('internal/safe_globals');
+
+constkSerializedError=0;
+constkSerializedObject=1;
+constkInspectedError=2;
+
+constGetPrototypeOf=Object.getPrototypeOf;
+constGetOwnPropertyDescriptor=Object.getOwnPropertyDescriptor;
+constGetOwnPropertyNames=Object.getOwnPropertyNames;
+constDefineProperty=Object.defineProperty;
+constAssign=Object.assign;
+constObjectPrototypeToString=
+Function.prototype.call.bind(Object.prototype.toString);
+constForEach=Function.prototype.call.bind(Array.prototype.forEach);
+constCall=Function.prototype.call.bind(Function.prototype.call);
+
+consterrors={
+ Error, TypeError, RangeError, URIError, SyntaxError, ReferenceError, EvalError
+};
+consterrorConstructorNames=newSafeSet(Object.keys(errors));
+
+functionTryGetAllProperties(object,target=object){
+constall=Object.create(null);
+if(object===null)
+returnall;
+Assign(all,TryGetAllProperties(GetPrototypeOf(object),target));
+constkeys=GetOwnPropertyNames(object);
+ForEach(keys,(key)=>{
+constdescriptor=GetOwnPropertyDescriptor(object,key);
+constgetter=descriptor.get;
+if(getter&&key!=='__proto__'){
+try{
+descriptor.value=Call(getter,target);
+}catch{}
+}
+if('value'indescriptor&&typeofdescriptor.value!=='function'){
+deletedescriptor.get;
+deletedescriptor.set;
+all[key]=descriptor;
+}
+});
+returnall;
+}
+
+functionGetConstructors(object){
+constconstructors=[];
+
+for(varcurrent=object;
+current!==null;
+current=GetPrototypeOf(current)){
+constdesc=GetOwnPropertyDescriptor(current,'constructor');
+if(desc&&desc.value){
+DefineProperty(constructors,constructors.length,{
+value: desc.value,enumerable: true
+});
+}
+}
+
+returnconstructors;
+}
+
+functionGetName(object){
+constdesc=GetOwnPropertyDescriptor(object,'name');
+returndesc&&desc.value;
+}
+
+letutil;
+functionlazyUtil(){
+if(!util)
+util=require('util');
+returnutil;
+}
+
+functionserializeError(error){
+try{
+if(typeoferror==='object'&&
+ObjectPrototypeToString(error)==='[object Error]'){
+constconstructors=GetConstructors(error);
+for(vari=constructors.length-1;i>=0;i--){
+constname=GetName(constructors[i]);
+if(errorConstructorNames.has(name)){
+try{error.stack;}catch{}
+constserialized=serialize({
+constructor: name,
+properties: TryGetAllProperties(error)
+});
+returnBuffer.concat([Buffer.from([kSerializedError]),serialized]);
+}
+}
+}
+}catch{}
+try{
+constserialized=serialize(error);
+returnBuffer.concat([Buffer.from([kSerializedObject]),serialized]);
+}catch{}
+returnBuffer.concat([Buffer.from([kInspectedError]),
+Buffer.from(lazyUtil().inspect(error),'utf8')]);
+}
+
+functiondeserializeError(error){
+switch(error[0]){
+casekSerializedError:
+const{ constructor, properties }=deserialize(error.subarray(1));
+constctor=errors[constructor];
+returnObject.create(ctor.prototype,properties);
+casekSerializedObject:
+returndeserialize(error.subarray(1));
+casekInspectedError:
+constbuf=Buffer.from(error.buffer,
+error.byteOffset+1,
+error.byteLength-1);
+returnbuf.toString('utf8');
+}
+require('assert').fail('This should not happen');
+}
+
+module.exports={ serializeError, deserializeError };

lib/internal/worker.js

@@ -1,6 +1,5 @@
 'use strict';
 
-constBuffer=require('buffer').Buffer;
 constEventEmitter=require('events');
 constassert=require('assert');
 constpath=require('path');
@@ -17,6 +16,7 @@ const{internalBinding } = require('internal/bootstrap/loaders');
 const{ MessagePort, MessageChannel }=internalBinding('messaging');
 const{ handle_onclose }=internalBinding('symbols');
 const{ clearAsyncIdStack }=require('internal/async_hooks');
+const{ serializeError, deserializeError }=require('internal/error-serdes');
 
 util.inherits(MessagePort,EventEmitter);
 
@@ -453,17 +453,6 @@ function setupChild(evalScript){
 }
 }
 
-// TODO(addaleax): These can be improved a lot.
-functionserializeError(error){
-returnBuffer.from(util.inspect(error),'utf8');
-}
-
-functiondeserializeError(error){
-returnBuffer.from(error.buffer,
-error.byteOffset,
-error.byteLength).toString('utf8');
-}
-
 functionpipeWithoutWarning(source,dest){
 constsourceMaxListeners=source._maxListeners;
 constdestMaxListeners=dest._maxListeners;

node.gyp

@@ -102,6 +102,7 @@
 'lib/internal/constants.js',
 'lib/internal/encoding.js',
 'lib/internal/errors.js',
+'lib/internal/error-serdes.js',
 'lib/internal/fixed_queue.js',
 'lib/internal/freelist.js',
 'lib/internal/fs/promises.js',

test/parallel/test-error-serdes.js

@@ -0,0 +1,46 @@
+// Flags: --expose-internals
+'use strict';
+require('../common');
+constassert=require('assert');
+const{ERR_INVALID_ARG_TYPE}=require('internal/errors').codes;
+const{ serializeError, deserializeError }=require('internal/error-serdes');
+
+functioncycle(err){
+returndeserializeError(serializeError(err));
+}
+
+assert.strictEqual(cycle(0),0);
+assert.strictEqual(cycle(-1),-1);
+assert.strictEqual(cycle(1.4),1.4);
+assert.strictEqual(cycle(null),null);
+assert.strictEqual(cycle(undefined),undefined);
+assert.strictEqual(cycle('foo'),'foo');
+
+{
+consterr=cycle(newError('foo'));
+assert(errinstanceofError);
+assert.strictEqual(err.name,'Error');
+assert.strictEqual(err.message,'foo');
+assert(/^Error:foo\n/.test(err.stack));
+}
+
+assert.strictEqual(cycle(newRangeError('foo')).name,'RangeError');
+assert.strictEqual(cycle(newTypeError('foo')).name,'TypeError');
+assert.strictEqual(cycle(newReferenceError('foo')).name,'ReferenceError');
+assert.strictEqual(cycle(newURIError('foo')).name,'URIError');
+assert.strictEqual(cycle(newEvalError('foo')).name,'EvalError');
+assert.strictEqual(cycle(newSyntaxError('foo')).name,'SyntaxError');
+
+classSubErrorextendsError{}
+
+assert.strictEqual(cycle(newSubError('foo')).name,'Error');
+
+assert.deepStrictEqual(cycle({message: 'foo'}),{message: 'foo'});
+assert.strictEqual(cycle(Function),'[Function: Function]');
+
+{
+consterr=newERR_INVALID_ARG_TYPE('object','Object',42);
+assert(/^TypeError\[ERR_INVALID_ARG_TYPE\]:/.test(err));
+assert.strictEqual(err.name,'TypeError [ERR_INVALID_ARG_TYPE]');
+assert.strictEqual(err.code,'ERR_INVALID_ARG_TYPE');
+}

test/parallel/test-worker-uncaught-exception-async.js

@@ -10,8 +10,7 @@ if (!process.env.HAS_STARTED_WORKER){
 constw=newWorker(__filename);
 w.on('message',common.mustNotCall());
 w.on('error',common.mustCall((err)=>{
-// TODO(addaleax): be more specific here
-assert(/foo/.test(err));
+assert(/^Error:foo$/.test(err));
 }));
 }else{
 setImmediate(()=>{

test/parallel/test-worker-uncaught-exception.js

@@ -10,8 +10,7 @@ if (!process.env.HAS_STARTED_WORKER){
 constw=newWorker(__filename);
 w.on('message',common.mustNotCall());
 w.on('error',common.mustCall((err)=>{
-// TODO(addaleax): be more specific here
-assert(/foo/.test(err));
+assert(/^Error:foo$/.test(err));
 }));
 }else{
 thrownewError('foo');