Conversation
| if (!data) { | ||
| throw new Error( | ||
| `StreamProvider - Unknown response id for response: ${response}` | ||
| ) | ||
| } |
There was a problem hiding this comment.
is this just a formatting preference or was something actually broken here?
There was a problem hiding this comment.
formatting preference. i can revert this and make the guard i added match the same syntax style if preferred
There was a problem hiding this comment.
No its alright, I think we prefer to always use brackets, I was just making sure I wasn't missing something obvious
|
Just to clarify, does this interface not support JSON-RPC notifications at all? I don't see how those would be received - this code is assuming each message written to this stream is a response. Though I don't see how we'd have received notifications previously either. |
|
Correct, this interface does not support JSON-RPC notifications at all. They would not be received before (unless maybe if the wallet UI made a So apologies, I see a way on the extension where I can avoid sending the notification to internal connections that use this specific provider and the changes in this PR should no longer be necessary |
Fixes unhandled case where a notification (response without id) is received by the
web3-stream-provider. This makes the request/response handling more closely match that of the inpage provider