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.

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

Add support for throwing errors with custom error code #272

Merged
merged 4 commits into from
Feb 21, 2024

Conversation

palamccc
Copy link

@palamccc palamccc commented Feb 17, 2024

Motivation:

In the rsocket-js implementation (v0 branch), all stream errors are serialized and transmitted to other end with fixed error code. Due to this bug, there is no way to send custom error codes over stream (Bug report #218)

Custom error codes are supported in the rsocket protocol and implemented in rsocket-java this using RSocketErrorException . When the application throws RSocketErrorExecption, the error code of the exception is sent over the stream. But this feature is not implemented in rsocket-js.

Modifications:

  • This PR adds custom Javascript Error class RSocketError in RSocketError.js.
  • When the application throws error of type RSocketError, the errorCode of the error object is sent over the stream.

Result:

This will allow application to send custom error codes to other party.

@palamccc palamccc changed the title Add support for throwing custom errors in streams Add support for throwing errors with custom error code Feb 17, 2024
@viglucci
Copy link
Member

viglucci commented Feb 19, 2024

Hey @palamccc thanks for the contribution!

Would you be able to add tests for your changes? Looks like RSocketServer-test.js might have good examples to build off of.

https://github.com/rsocket/rsocket-js/blob/master/packages/rsocket-core/src/__tests__/RSocketServer-test.js#L144-L151

Additionally the DCO check will ideally need to be resolved before this can be merged.

@@ -943,3 +943,11 @@ function deserializeMetadataPushPayload<D, M>(
metadata: serializers.metadata.deserialize(frame.metadata),
};
}

export class RSocketError extends Error {
Copy link
Member

Choose a reason for hiding this comment

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

This might be best as its own file RSocketError.js.

Copy link
Author

Choose a reason for hiding this comment

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

Done 👍🏽

Signed-off-by: palani <palam.c@gmail.com>
Signed-off-by: palani <palam.c@gmail.com>
Signed-off-by: palani <palam.c@gmail.com>
Signed-off-by: palani <palam.c@gmail.com>
@palamccc palamccc force-pushed the palamcc-v0-custom-error branch from 227ccf9 to cd30156 Compare February 20, 2024 03:18
@palamccc palamccc requested a review from viglucci February 20, 2024 03:23
@palamccc
Copy link
Author

Would you be able to add tests for your changes? Looks like RSocketServer-test.js might have good examples to build off of.

@viglucci I've added tests now.

@viglucci
Copy link
Member

viglucci commented Feb 21, 2024

LGTM. Thanks! I'll LYK when a new version is published.

@viglucci viglucci merged commit 860ddd5 into rsocket:master Feb 21, 2024
5 checks passed
@palamccc
Copy link
Author

palamccc commented Feb 28, 2024

I've contributed types to DefinitelyTyped DefinitelyTyped/DefinitelyTyped#68788
Waiting for 0.0.28 release :-).

@viglucci
Copy link
Member

viglucci commented Mar 6, 2024

@palamccc I've published a new version which should include this change. If you would be so kind, please validate this change with rsocket-core@0.0.29-alpha.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants