Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: improve preparePushMessage #1509

Merged
merged 8 commits into from
Aug 30, 2023
Merged

chore: improve preparePushMessage #1509

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a1828ee
chore: improve readability a bit
weboko Aug 28, 2023
932b5ef
rename
weboko Aug 28, 2023
456dc09
decople to a type
weboko Aug 28, 2023
ede9008
Merge branch 'master' into weboko/lightpush-send
weboko Aug 28, 2023
e99272f
Merge branch 'master' of github.com:waku-org/js-waku into weboko/ligh…
weboko Aug 29, 2023
7a2f6b4
Merge branch 'weboko/lightpush-send' of github.com:waku-org/js-waku i…
weboko Aug 29, 2023
c4c3025
fix error
weboko Aug 29, 2023
c521ff5
Merge branch 'master' into weboko/lightpush-send
weboko Aug 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 39 additions & 35 deletions packages/core/src/lib/light_push/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ const log = debug("waku:light-push");
export const LightPushCodec = "/vac/waku/lightpush/2.0.0-beta1";
export { PushResponse };

type PreparePushMessageResult =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can move this to @waku/interfaces

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can move this to @waku/interfaces

Hum, not sure I agree. preparepush is not really part of the push protocol and it's an internal type.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo all interfaces/types should exist in @waku/interfaces

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's meant to store common interfaces, not all types.

Copy link
Collaborator

@danisharora099 danisharora099 Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd suggest to have an internal types folder in the monorepo.. helps maintain readability and structure

Copy link
Collaborator Author

@weboko weboko Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am against moving all types to @waku/interfaces and I think it is better to dedicate it for shared between packages/public types.

PreparePushMessageResult I would perceive as a private local variable so it makes sense to keep only within the file.

| {
query: PushRpc;
error: null;
}
| {
query: null;
error: SendError;
};

/**
* Implements the [Waku v2 Light Push protocol](https://rfc.vac.dev/spec/19/).
*/
Expand All @@ -42,26 +52,32 @@ class LightPush extends BaseProtocol implements ILightPush {
encoder: IEncoder,
message: IMessage,
pubSubTopic: string
): Promise<{
query: PushRpc | null;
error?: SendError;
}> {
if (!isSizeValid(message.payload)) {
log("Failed to send waku light push: message is bigger than 1MB");
return { query: null, error: SendError.SIZE_TOO_BIG };
}
): Promise<PreparePushMessageResult> {
try {
if (!isSizeValid(message.payload)) {
log("Failed to send waku light push: message is bigger than 1MB");
return { query: null, error: SendError.SIZE_TOO_BIG };
}

const protoMessage = await encoder.toProtoObj(message);
if (!protoMessage) {
log("Failed to encode to protoMessage, aborting push");
return {
query: null,
error: SendError.ENCODE_FAILED
};
}

const query = PushRpc.createRequest(protoMessage, pubSubTopic);
return { query, error: null };
} catch (error) {
log("Failed to prepare push message", error);

const protoMessage = await encoder.toProtoObj(message);
if (!protoMessage) {
log("Failed to encode to protoMessage, aborting push");
return {
query: null,
error: SendError.ENCODE_FAILED
error: SendError.GENERIC_FAIL
};
}

const query = PushRpc.createRequest(protoMessage, pubSubTopic);
return { query };
}

async send(
Expand All @@ -71,33 +87,21 @@ class LightPush extends BaseProtocol implements ILightPush {
): Promise<SendResult> {
const { pubSubTopic = DefaultPubSubTopic } = this.options;
const recipients: PeerId[] = [];
let error: undefined | SendError = undefined;

let query: PushRpc | null = null;

try {
const { query: preparedQuery, error: preparationError } =
await this.preparePushMessage(encoder, message, pubSubTopic);

if (preparationError) {
return {
recipients,
error: preparationError
};
}

query = preparedQuery;
} catch (error) {
log("Failed to prepare push message", error);
}
const { query, error: preparationError } = await this.preparePushMessage(
encoder,
message,
pubSubTopic
);

if (!query) {
if (preparationError || !query) {
return {
recipients,
error: SendError.GENERIC_FAIL
error: preparationError
};
}

let error: undefined | SendError = undefined;
const peer = await this.getPeer(opts?.peerId);
const stream = await this.newStream(peer);

Expand Down
Loading