GRPC errors are not being surfaced

[mkobetic]

I've confirmed that if we get a GRPC error in response to a publish request to the messaging API, e.g.:

2022-11-18 14:41:27 2022-11-18T19:41:27.645Z ERROR xmtpd.api api request {"node": "16Uiu2HAmNCxLZCkXNbpVPBpSSnHj9iq4HZQj7fxRzw2kj1kKSHHA", "service": "xmtp.message_api.v1.MessageApi", "method": "Publish", "client": "xmtp-js", "client_version": "xmtp-js/0.0.0-development", "app": "", "app_version": "", "client_ip": "172.24.0.1", "error": "rpc error: code = PermissionDenied desc = publishing to restricted topic", "error_code": "PermissionDenied", "error_message": "publishing to restricted topic"}

The [MessageApi.Publish]() will simply return the PublishResponse which is just an emtpy object, just like with a successful publish response. I suspect this is a problem across the whole API, that the client isn't getting any exceptions thrown in response to at least the GRPC errors.

Consequently the retry code that we have is likely not functioning as we thought. Similarly token refreshing may be broken too.

I suspect the problem might be with the generated GRPC client. The fetchReq function looks like this:

export function fetchReq<I, O>(path: string, init?: InitReq): Promise<O> {
  const {pathPrefix, ...req} = init || {}

  const url = pathPrefix ? `${pathPrefix}${path}` : path

  return fetch(url, req).then(r => r.json().then((body: O) => {
    if (!r.ok) { throw body; }
    return body;
  })) as Promise<O>
}

[neekolas]

You could verify this by just taking down your local API and running the test suite. I don't know if the problem is general, but there is a specific issue with the publish function in the SDK (we have always been ignoring errors there from when we were using LibP2P and it was fire and forget), but since the move to GRPC actual errors are possible.

[neekolas]

The r.ok flag will be false if the HTTP status code is not 2XX. https://www.tjvantoll.com/2015/09/13/fetch-and-errors/

[mkobetic]

Yeah, but I don't see anything throwing in response to the GRPC error, maybe the gateway doesn't set the status the way we expect? In the go client in node-go we compare strictly to 200. I'll dig in some more.

[neekolas]

This is the specific line I am referring to https://github.com/xmtp/xmtp-js/blob/main/src/Client.ts#L313

[mkobetic]

Hm, on the node side I'm seeing this delivered to the http client, response status 403 Forbidden and body

"{\"code\":7,\"message\":\"publishing to restricted topic\",\"details\":[]}"

[mkobetic]

This is the specific line I am referring to https://github.com/xmtp/xmtp-js/blob/main/src/Client.ts#L313

Ah OK, I was looking at https://github.com/xmtp/xmtp-js/blob/main/src/store/PrivateTopicStore.ts#L37 which goes straight to the ApiClient. I debugged it down into the client code and nothing seems to be thrown. 🤷 I'm sure I can at least write some tests for this.

[mkobetic]

OK, I was wrong the GRPC errors are throwing. What threw me off (:cringe:) was that the failing network store test seemingly wasn't throwing on the failing publish, but it actually was, it was the test case that wasn't awaiting the set calls.

There seems to also be a fair bit of confusion in the test suite about how jest works with async code and the interactions between rejects and toThrow. For example, IIUC rejects.toThrow doesn't make sense which we seem to have a fair bit of in the test suite. There is this key bit in jest docs that is important to keep in mind:

Be sure to return (or await) the promise - if you omit the return/await statement, your test will complete before the promise returned from fetchData resolves or rejects.

There are probably few awaits/returns missing in front of some of the expects in the test suite.

[mkobetic]

OK. looks like I was wrong about rejects.toThrow as well. It works fine if an Error is thrown, but not if something else is thrown. GRPC errors are thrown as object {"code": 7, "details": [], "message": "publishing to restricted topic"}, so toThrow doesn't work for that. The assertions failure Received function did not throw was another thing that threw me off for quite a bit.

[mkobetic]

Looks like the bit from the jest docs that I quoted above is BS as well. Tests seem to be working fine without the awaits/returns as well.

[mkobetic]

For some extra confusion it seems that toThrow works for non Errors in the synchronous use case. This passes

describe('scratch', () => {
  it('blows', () => {
    expect(() => {
      throw { mud: 7 }
    }).toThrow(Object)
  })
})

[github-actions[bot]]

:tada: This issue has been resolved in version 7.1.3 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: