feat!: return response interface for success/error #1725
Conversation
danisharora099
changed the title
size-limit report 📦
|
danisharora099
force-pushed
the
feat/filter-response-interface
branch
from
55763e7 to
5ca284a
Compare
| return { | ||
| requestId: requestId, | ||
| statusCode: statusCode | ||
| }; | ||
| } catch (e) { | ||
| throw new Error( |
There was a problem hiding this comment.
There are still a few errors being thrown here and line 109. They should also be converted. Ideally nothing is throw and users entirely rely on the enum error
| @@ -72,7 +73,7 @@ class Subscription { | |||
| async subscribe<T extends IDecodedMessage>( | |||
| decoders: IDecoder<T> | IDecoder<T>[], | |||
| callback: Callback<T> | |||
| ): Promise<void> { | |||
| ): Promise<IFilterResponse> { | |||
There was a problem hiding this comment.
This function can fail in several manners:
- locally (can't encode, other checks fail)
- on the communication (can't open stream, remote node does not reply)
- Remote node send an error back
All these cases should be covered with an Enum
Only in the case of (3) then you may want to return a "sub error", ie, what the remote peer returned.
Currently, this is still throwing exception on (1) and (2). It should not.
Finally, there is an opportunity to have common errors with light push. SendError should be re-used here and made more generic.
Do not we will change light push to also return error like filter, so while it's not there yet, knowing that should allow a common design. E.g. today light push remote peer failure does not include sub error, tomorrow it will.
| ); | ||
|
|
||
| if (!res || !res.length) { | ||
| throw Error( |
|
will be contd after #1682 as it involves a significant refactor (+ dispatch errors) |
|
@danisharora099 Please ping me or reset my review when ready to re-review. |
|
superseded by #1694 |
For the parent issue: #1694, we want to structure all protocols to return error codes & enums over throwing exceptions.
This PR:
IFilterResponsewhich includesstatusCode, message, requestIdNotes: