API: propagate errors to single-response APIs

packages/aragon-rpc-messenger/src/index.js

@@ -2,7 +2,7 @@ import jsonrpc from './jsonrpc'
 import MessagePortMessage from './providers/MessagePortMessage'
 import WindowMessage from './providers/WindowMessage'
 import DevMessage from './providers/DevMessage'
-import { first, filter } from 'rxjs/operators'
+import { first, filter, map } from 'rxjs/operators'
 
 export const providers = {
   MessagePortMessage,
@@ -91,7 +91,14 @@ export default class Messenger {
     const id = this.send(method, params)
 
     return this.responses().pipe(
-      filter((message) => message.id === id)
+      filter((message) => message.id === id),
+      map((response) => {
+        if (response.error) {
+          response.error = new Error(response.error)
+        }
+        return response
+      })
+      // Let callers handle errors themselves
     )
   }
 
@@ -104,7 +111,14 @@ export default class Messenger {
    */
   sendAndObserveResponse (method, params = []) {
     return this.sendAndObserveResponses(method, params).pipe(
-      first()
+      first(),
+      map((response) => {
+        // Emit an error if the response is an error
+        if (response.error) {
+          throw response.error
+        }
+        return response
+      })
     )
   }
 }

packages/aragon-rpc-messenger/src/index.test.js

@@ -69,13 +69,14 @@ test('should encode and send the request', t => {
 })
 
 test('should filter the incoming messages to responses only', (t) => {
+  t.plan(2)
+
   // arrange
   const busMock = new Subject()
   const instance = new Messenger(null)
   instance.bus = () => busMock
   jsonrpcStub.isValidResponse = sinon.stub().returns(true)
-  // assert
-  t.plan(2)
+
   // act
   const messages = instance.responses()
   messages.subscribe(value => {
@@ -87,13 +88,14 @@ test('should filter the incoming messages to responses only', (t) => {
 })
 
 test('should filter the incoming messages to requests only', (t) => {
+  t.plan(2)
+
   // arrange
   const busMock = new Subject()
   const instance = new Messenger(null)
   instance.bus = () => busMock
   jsonrpcStub.isValidResponse = sinon.stub().returns(false)
-  // assert
-  t.plan(2)
+
   // act
   const messages = instance.requests()
   messages.subscribe(value => {
@@ -105,38 +107,89 @@ test('should filter the incoming messages to requests only', (t) => {
 })
 
 test('should send and observe responses', (t) => {
+  t.plan(4)
+
   // arrange
   const instance = new Messenger(null)
   sinon.stub(instance, 'send').returns(41)
   sinon.stub(instance, 'responses').returns(new Subject())
+  const messages = instance.sendAndObserveResponses('sendEth')
+
   // assert
-  t.plan(4)
+  messages.subscribe(value => t.is(value.data, 'thanks'))
+  t.is(instance.send.getCall(0).args[0], 'sendEth')
+  t.deepEqual(instance.send.getCall(0).args[1], [])
+
   // act
+  instance.responses().next({ data: 'thanks', id: 41 })
+  instance.responses().next({ data: 'thanks', id: 41 })
+})
+
+test('should send and observe responses, even if errors are included', (t) => {
+  t.plan(6)
+
+  // arrange
+  const instance = new Messenger(null)
+  sinon.stub(instance, 'send').returns(41)
+  sinon.stub(instance, 'responses').returns(new Subject())
   const messages = instance.sendAndObserveResponses('sendEth')
-  messages.subscribe(value => t.is(value.data, 'thanks'))
+
   // assert
+  messages.subscribe(value => {
+    if (value.data) {
+      t.is(value.data, 'thanks')
+    } else if (value.error) {
+      t.true(value.error instanceof Error)
+      t.is(value.error.message, 'no thanks')
+    }
+  })
   t.is(instance.send.getCall(0).args[0], 'sendEth')
   t.deepEqual(instance.send.getCall(0).args[1], [])
 
+  // act
   instance.responses().next({ data: 'thanks', id: 41 })
+  instance.responses().next({ error: 'no thanks', id: 41 })
   instance.responses().next({ data: 'thanks', id: 41 })
 })
 
 test('should send and observe only the first response', (t) => {
+  t.plan(3)
+
   // arrange
   const instance = new Messenger(null)
   sinon.stub(instance, 'sendAndObserveResponses').returns(new Subject())
-  // assert
-  t.plan(3)
+
   // act
   const messages = instance.sendAndObserveResponse('sendAnt')
   // assert
   messages.subscribe(value => t.is(value, 'first'))
-
   t.is(instance.sendAndObserveResponses.getCall(0).args[0], 'sendAnt')
   t.deepEqual(instance.sendAndObserveResponses.getCall(0).args[1], [])
 
   instance.sendAndObserveResponses().next('first')
   instance.sendAndObserveResponses().next('second')
   instance.sendAndObserveResponses().next('third')
 })
+
+test('should send and observe only the first error', (t) => {
+  t.plan(4)
+
+  // arrange
+  const instance = new Messenger(null)
+  sinon.stub(instance, 'sendAndObserveResponses').returns(new Subject())
+  const messages = instance.sendAndObserveResponse('sendAnt')
+  // assert
+  messages.subscribe(
+    value => t.fail('should not have emitted any next values'),
+    error => {
+      t.true(error instanceof Error)
+      t.is(error.message, 'bad first')
+    }
+  )
+  t.is(instance.sendAndObserveResponses.getCall(0).args[0], 'sendAnt')
+  t.deepEqual(instance.sendAndObserveResponses.getCall(0).args[1], [])
+
+  // act
+  instance.sendAndObserveResponses().next({ error: new Error('bad first') })
+  instance.sendAndObserveResponses().next('second')
+})

packages/aragon-wrapper/src/rpc/handlers/index.js

@@ -6,8 +6,8 @@ export function createResponse ({ request: { id } }, { error, value = null, kind
     return {}
   }
 
-  if (error) {
-    return { id, payload: error }
+  if (kind === 'E') {
+    return { id, payload: error || new Error() }
   }
 
   return { id, payload: value }
@@ -34,7 +34,7 @@ export function createRequestHandler (request$, requestType, handler) {
       createResponse
     ),
     // filter empty responses caused by Notifications of kind 'C'
-    filter((response) => response.payload !== undefined || response.error !== undefined)
+    filter((response) => response.payload !== undefined)
   )
 }
 

packages/aragon-wrapper/src/rpc/handlers/index.test.js

@@ -12,7 +12,7 @@ test.afterEach.always(() => {
 })
 
 test('should create a request handler', async (t) => {
-  t.plan(4)
+  t.plan(5)
   // arrange
   const requestStub = from([{
     request: {
@@ -33,6 +33,12 @@ test('should create a request handler', async (t) => {
       params: ['set', 'settings'],
       value: { foo: 'bar' }
     }
+  }, {
+    request: {
+      id: 'uuid5',
+      method: 'cache',
+      params: ['clear']
+    }
   }, {
     request: {
       id: 'uuid6',
@@ -56,14 +62,21 @@ test('should create a request handler', async (t) => {
       return Promise.reject(new Error(`no permissions to change ${request.params[1]}!!`))
     }
 
+    if (request.params[0] === 'clear') {
+      return Promise.reject(new Error())
+    }
+
     return Promise.resolve(`resolved ${request.params[1]}`)
   }
   // act
   const result = createRequestHandler(requestStub, 'cache', handlerStub)
   // assert
   result.subscribe(value => {
     if (value.id === 'uuid1') return t.is(value.payload, 'resolved settings')
+    if (value.id === 'uuid4') return t.true(value.payload instanceof Error)
     if (value.id === 'uuid4') return t.is(value.payload.message, 'no permissions to change settings!!')
+    if (value.id === 'uuid5') return t.true(value.payload instanceof Error)
+    if (value.id === 'uuid5') return t.is(value.payload.message, '')
     if (value.id === 'uuid6') return t.is(value.payload, 'resolved profile')
     if (value.id === 'uuid8') return t.is(value.payload, null)
   })