Skip to content

Comments

feat(binding/nodejs): add TimeoutLayer, LoggingLayer and ThrottleLayer in nodejs binding#6772

Merged
Xuanwo merged 3 commits intoapache:mainfrom
Kilerd:feat/add-layers-in-nodejs
Nov 14, 2025
Merged

feat(binding/nodejs): add TimeoutLayer, LoggingLayer and ThrottleLayer in nodejs binding#6772
Xuanwo merged 3 commits intoapache:mainfrom
Kilerd:feat/add-layers-in-nodejs

Conversation

@Kilerd
Copy link
Contributor

@Kilerd Kilerd commented Nov 11, 2025

Which issue does this PR close?

no related issue

Rationale for this change

  • add TimeoutLayer, LoggingLayer and ThrottleLayer in nodejs binding

Are there any user-facing changes?

@Kilerd Kilerd force-pushed the feat/add-layers-in-nodejs branch from 039ea72 to 92f8c09 Compare November 11, 2025 10:47
@Kilerd Kilerd force-pushed the feat/add-layers-in-nodejs branch from 92f8c09 to 77d6e02 Compare November 11, 2025 14:40
@Kilerd Kilerd marked this pull request as ready for review November 11, 2025 16:46
@Kilerd Kilerd requested a review from suyanhanx as a code owner November 11, 2025 16:46
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels Nov 11, 2025
#[napi(constructor)]
pub fn new(bandwidth: u32, burst: u32) -> Self {
if bandwidth == 0 {
panic!("bandwidth must be greater than 0");
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this panic here?

panic!("bandwidth must be greater than 0");
}
if burst == 0 {
panic!("burst must be greater than 0");
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto I search the node binding, seems no this kind of check

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I suggest we just pass all value into rust core and don't add our own logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the redundant checks

Copy link
Contributor

@yihong0618 yihong0618 left a comment

Choose a reason for hiding this comment

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

LGTM

@Xuanwo Xuanwo merged commit fc5725a into apache:main Nov 14, 2025
63 checks passed
@Kilerd Kilerd deleted the feat/add-layers-in-nodejs branch November 14, 2025 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/feat The PR implements a new feature or has a title that begins with "feat" size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants