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

[rpc module]: return error from subscriptions to make error handling more ergonomic #734

Closed
Tracked by #736
niklasad1 opened this issue Apr 21, 2022 · 0 comments · Fixed by #799
Closed
Tracked by #736
Assignees

Comments

@niklasad1
Copy link
Member

niklasad1 commented Apr 21, 2022

Recently the subscription code got refactor where one must reject or accept a subscription manually and because
SubscriptionClosed is not treated as an error the closures must not return Result.

However, it's quite awkward to so:

Example:

	module
		.register_subscription("sub_one_param", "sub_one_param", "unsub_one_param", |params, pending, _| {
		       // not possible to return Result here.
                       let idx = match params.one::<usize>() {
				Ok(idx) => (idx, sink),
				_ => {
                                    pending.reject("some special error);
                                    return;
                               }
			};
		       
                      let sink = match pending.accept() {
                         Some(sink) => sink,
                         None => return;
                      };

                      sink.pipe_from_stream(...);
		})
		.unwrap();

So it would be more ergonomic / less awkward if it would be possible to do:

	module
		.register_subscription("sub_one_param", "sub_one_param", "unsub_one_param", |params, pending, _| {
		       // pending implement drop and will reject the subscription with default error
                       let idx = params.one()?;
                       let sink = pending.accept()?;
		       
                        sink.pipe_from_stream(...);
                        Ok(())
		})
		.unwrap();

The tricky part is that if one wants to reject a call explicitly then the error type has to be cloned or borrowed.
So that's rationale why the API is like is at the moment.

@niklasad1 niklasad1 mentioned this issue Apr 22, 2022
9 tasks
@jsdw jsdw added this to the v1.0 milestone Jun 17, 2022
@lexnv lexnv self-assigned this Jun 17, 2022
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 a pull request may close this issue.

3 participants