Aaronius / penpal

A promise-based library for securely communicating with iframes via postMessage.
MIT License
389 stars 56 forks source link

Penpal should be capable of transferring Error objects as rejection reasons #15

Closed jrencz closed 6 years ago

jrencz commented 6 years ago

As MDN Page on Promise.reject mentions rejecting promises with Error instances is a good practice.

There's even a prefer-promise-reject-errors ESLint rule for that.

Penpal fails with

index.js:234 Uncaught (in promise) DOMException: Failed to execute 'postMessage' on 'Window': Error: <error message here> could not be cloned.
    at http://localhost:3100/vendor.bundle.js:45082:21
    at <anonymous>

It should be capable of handling Error instances. I'd expect for either of those to happen instead of getting the DOMException thrown:

jrencz commented 6 years ago

It can be handled in project like:

const unwrapRejectionReasonsFromErrors = methods => Object
  .keys(methods)
  .reduce((collector, methodName) => Object.assign(collector, {
    [methodName]: (...args) => Promise
      .resolve(methods[methodName](...args))
      .catch(error => Promise.reject(error instanceof Error ?
        `[From Penpal child]: ${ error.message }` :
        error
      )),
  }), {});
Penpal.connectToParent({
  methods: unwrapRejectionReasonsFromErrors({
    foo: () => Promise.reject(new Error('foo failed')),
  }),
});

In parent if foo is called one will see Uncaught (in promise) [From Penpal child]: foo failed

In fact this may be incorporated into Penpal source, yet with configurable prefix

Aaronius commented 6 years ago

I've published this on the next dist-tag as 3.0.0-alpha.4. You can try it out via npm install penpal@next or npm install penpal@3.0.0-alpha.4.

Give it a shot and let me know how it works for you. I took the "powerful" route of serializing/deserializing the error object. The name, message, and stack properties will make it through the serialization process.

Aaronius commented 6 years ago

BTW, I only serialize/deserialize errors that are thrown or are used to reject promises. In other words, if a method returns an error or returns a promise that's resolved with an error, I don't attempt to serialize the error (in fact, I go out of my way to not serialize it). I wasn't sure which way I should go on that but I'm curious to know what you think.

jrencz commented 6 years ago

Sounds great.

I can't think of a reasonable case where passing an error as return/resolve value through the iframe boundary is more desirable than throwing it. Especially that the other side can always catch and recover (return from then) if for some reason someone needs it.

jrencz commented 6 years ago

It looks like it works on 3.0.0-alpha.5

Aaronius commented 6 years ago

Published in 3.0.0.