feat(metadata): use error codes #1904
Conversation
size-limit report 📦
|
| ShardingParams | ||
| } from "./protocols.js"; | ||
|
|
||
| export type QueryResult = |
There was a problem hiding this comment.
nit: it should be generic and should cover PreparePushMessageResult as well
proposed structure:
// Most common generic to cover potential cases in future
type Result<T, E> =
| {
query: T;
error: null;
}
| {
query: null;
error: E;
};
export type ProtocolResult = Result<ShardInfo, ProtocolError>; // or QueryResult There was a problem hiding this comment.
The generic should already be able to cover that. Note: this PR is only for metadata and LightPush will be handled in a separate PR. Thanks for the flag though!
| @@ -101,7 +101,7 @@ export type Callback<T extends IDecodedMessage> = ( | |||
| msg: T | |||
| ) => void | Promise<void>; | |||
|
|
|||
| export enum SendError { | |||
| export enum ProtocolError { | |||
There was a problem hiding this comment.
This one I'd like to discuss.
This enum is used as a constant and part of runtime and not typescript compile time.
That means that in some cases @waku/interfaces should be dependencies and not devDependencies.
So far we didn't see any issues because of that and I was not able to get any in my experiments, but overall it seems to me worth making a part of @waku/core.
wdyt @waku-org/js-waku-developers ?
There was a problem hiding this comment.
@weboko can we track this in an issue? We can discuss this in the coming PM call, or async within the issue
cc @adklempner
There was a problem hiding this comment.
sure, I wanted to bring this to discussion so that our team is aware of it
Problem
Protocols currently use different pattern to handle errors.
Light push uses error code while filter uses exception.
It is best for the user if a common pattern is used across the API.
Also, there is an opportunity to re-use error code across the codebase and have similar pattern on the user side to handle them
Solution
SendErrortoProtocolErrorto be more accomodatingNotes
Contribution checklist:
!in title if breaks public API;