Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error serialization throws #26

Open
nkbt opened this issue Aug 22, 2016 · 3 comments
Open

Error serialization throws #26

nkbt opened this issue Aug 22, 2016 · 3 comments

Comments

@nkbt
Copy link

nkbt commented Aug 22, 2016

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
Copy link
Author

nkbt commented Aug 22, 2016

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
Copy link
Owner

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
Copy link
Author

nkbt commented Aug 22, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants