search

glenjamin / transit-immutable-js

Transit serialisation for Immutable.js
MIT License
250 stars 31 forks source link

`Error` serialization throws #26

Open nkbt opened 8 years ago

nkbt commented 8 years ago

Not sure if it is needed in general, but I ran into this today.

11:16 $ node
> const {toJSON} = require('transit-immutable-js')
undefined
> toJSON({error: new Error('OMG')})
Error: Error serializing unrecognized object Error: OMG
    at transit.map.transit.makeWriteHandler.rep (/Users/nkbt/mh/ui/node_modules/transit-immutable-js/index.js:135:17)
    at Object.com.cognitect.transit.impl.writer.marshal (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2912:129)
    at Object.com.cognitect.transit.impl.writer.emitMap (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2875:115)
    at Object.com.cognitect.transit.impl.writer.marshal (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2932:50)
    at Object.com.cognitect.transit.impl.writer.marshalTop (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2950:59)
    at com.cognitect.transit.impl.writer.Writer.write [as write] (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2962:231)
    at toJSON (/Users/nkbt/mh/ui/node_modules/transit-immutable-js/index.js:203:21)
    at repl:1:1
    at sigintHandlersWrap (vm.js:22:35)
    at sigintHandlersWrap (vm.js:96:12)
>

transit-immutable-js falls back to "default" case"

    "default", transit.makeWriteHandler({
      tag: function() {
        return 'iM';
      },
      rep: function(m) {
        if (!('toMap' in m)) {
          var e = "Error serializing unrecognized object " + m.toString();
          throw new Error(e);
        }
        return mapSerializer(m.toMap());
      }
    })

To be fair, transit-js does not serialize Error either

> transit.writer('json').write({error: new Error('OMG')})
Error: Cannot write Error
    at Error (native)
    at Object.com.cognitect.transit.impl.writer.marshal (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2937:71)
    at Object.com.cognitect.transit.impl.writer.emitMap (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2875:115)
    at Object.com.cognitect.transit.impl.writer.marshal (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2932:50)
    at Object.com.cognitect.transit.impl.writer.marshalTop (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2950:59)
    at com.cognitect.transit.impl.writer.Writer.write [as write] (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2962:231)
    at repl:1:18
    at sigintHandlersWrap (vm.js:22:35)
    at sigintHandlersWrap (vm.js:96:12)
    at ContextifyScript.Script.runInThisContext (vm.js:21:12)

Feel free to close if out of scope.

nkbt commented 8 years ago

It might be sort of a problem since transit-immutable-js does not check if object has its own toJSON method

  CustomError.prototype.toJSON = function () {
    return {
      type: 'error',
      name: this.name,
      message: this.message,
      payload: this.payload
    };
  };
> JSON.stringify({err: new NotFoundError})
'{"err":{"type":"error","name":"NotFoundError","message":"Not Found","payload":{"status":404}}}'
> const trim = require('transit-immutable-js')
'use strict'
> trim.toJSON({err: new NotFoundError})
Error: Error serializing unrecognized object NotFoundError: Not Found
    at transit.map.transit.makeWriteHandler.rep (/Users/nkbt/mh/ui/node_modules/transit-immutable-js/index.js:135:17)
    at Object.com.cognitect.transit.impl.writer.marshal (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2912:129)
    at Object.com.cognitect.transit.impl.writer.emitMap (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2875:115)
    at Object.com.cognitect.transit.impl.writer.marshal (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2932:50)
    at Object.com.cognitect.transit.impl.writer.marshalTop (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2950:59)
    at com.cognitect.transit.impl.writer.Writer.write [as write] (/Users/nkbt/mh/ui/node_modules/transit-js/transit.js:2962:231)
    at Object.toJSON (/Users/nkbt/mh/ui/node_modules/transit-immutable-js/index.js:203:21)
    at repl:3:6
    at ContextifyScript.Script.runInThisContext (vm.js:25:33)
    at Object.exports.runInThisContext (vm.js:77:17)
>
glenjamin commented 8 years ago

I'm not sure if toJSON is the best choice, as there's no way to fromJSON.

It's probably worth exposing a way to register additional (de)serialisations for custom objects though.

nkbt commented 8 years ago

That really feels like out of scope. Also I reckon it might be quite unsafe feature to encode error without stripping away stack trace. So far my solution is to have error codec and use it in reducer (so errors are always kept in plain js objects). Would be interesting to hear what others do with it.

Allowing custom serializers might be a nice thing, unless it adds too much complexity of course.