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

Make rpc handlers abstract arrow function properties. #5

Closed
tegefaulkes opened this issue Mar 24, 2023 · 4 comments · Fixed by #8
Closed

Make rpc handlers abstract arrow function properties. #5

tegefaulkes opened this issue Mar 24, 2023 · 4 comments · Fixed by #8
Assignees
@tegefaulkes
Labels
development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management

Comments

@tegefaulkes
Copy link
Contributor

Specification

Currently the rpc handlers are abstract methods that need to be implemented. These should be an abstract arrow function property and handlers should be implemented as arrow functions. This will allow us to use them without rebinding the this context.

This should be done during the agent migration PR.

Additional context

Tasks

  1. Update the abstract handler's handle methods to be abstract arrow function properties.
@tegefaulkes tegefaulkes added the development Standard development label Mar 24, 2023
@tegefaulkes tegefaulkes self-assigned this Mar 24, 2023
@tegefaulkes tegefaulkes mentioned this issue Mar 24, 2023
Closed
@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management label Jul 9, 2023
This was referenced Jul 20, 2023
Merged
Merged
@CMCDragonkai CMCDragonkai transferred this issue from MatrixAI/Polykey Aug 18, 2023
@CMCDragonkai
Copy link
Member

This is moved here as updating this in js-rpc will force the adoption of it in Polykey when it updates.

@CMCDragonkai CMCDragonkai mentioned this issue Sep 5, 2023
Merged
31 tasks
@addievo
Copy link
Contributor

addievo commented Sep 13, 2023

Resolved in #8

@addievo addievo closed this as completed Sep 13, 2023
addievo added a commit that referenced this issue Sep 13, 2023
@addievo
feat: handlers implementations are now abstract arrow functions
45ac70e 
* Fixes #5

[ci skip]
@tegefaulkes
Copy link
Contributor Author

You need to re-open this. It should be closed automatically by #8 being merged.

@CMCDragonkai CMCDragonkai reopened this Sep 13, 2023
@CMCDragonkai
Copy link
Member

@addievo this issue is not fixed in your PR #8. You need to change the classes to specify that handlers should a be an arrow property and add tests into the RPC to be make it to be the case.

I'm not sure if class methods even satisfy arrow property types, and vice versa.

addievo added a commit that referenced this issue Sep 26, 2023
@addievo
Works as a standalone library now. Pending review to merge to staging.
f30da25 
callers and handlers are now refactored

* WIP - Newline now works, refers issue #1

node v20 fix

feat: handlers implementations are now abstract arrow functions

* Fixes #5

[ci skip]

* resolves issue 5, makes RPC handlers abstract arrow function properties

feat: rename to uppercase

[ci skip]

fix: handler export fix

[ci skip]

fix: tsconf from quic

[ci skip]

fix: dependencies (js quic), events and errors versions, changing to relative imports, jest dev dependency, js-quic tsconfig

[ci skip]

fix: tests imports, using @

[ci skip]

chore: removed sysexits

chore: fix default exports for callers and handlers
Fixed index for handlers

fix: remove @matrixai/id

fix: remove @matrixai/id and ix

chore : diagram

[ci skip]

chore : lintfix
fix: errors now extend AbstractError

[ci skip]

fix: undoing fix #1

[ci skip]

replacd errorCode with just code, references std error codes from rpc spec

feat: events based createDestroy

[ci skip]

chore: img format fix

[ci skip]

chore: img in README.md

[ci skip]

feat: allows the user to pass in a generator function if the user wishes to specify a particular id

[ci skip]

fix: fixes #7

* Removes graceTimer and related jests

chore: idGen name change. idGen parameter in creation and constructor. No longer optional. Only defaulted in one place.

wip: added idgen to jests, was missing.

[ci skip]

wip: reimported ix, since a few tests rely on it.
removed, matrixai/id

wip: jests for #4
removed, matrixai/id

wip: * Implements custom RPC Error codes.
     * Fixed jest for concurrent timeouts
     * All errors now have a cause
     * All errors now use custom error codes.

wip: *Client uses ctx timer now

wip: *Jests to test concurrency

wip: *custom RPC based errors for RPC Client, now all errors have a cause and an error message

WIP: * Refactor out sensitiveReplacer

WIP: * Refactor out sensitiveReplacer

WIP: * Update to latest async init and events
* set default timeout to Infinity
* jest to check server and client with infinite timeout
* fixing jests which broke after changing default timeout to infinity

WIP: f1x #4

WIP: f1x #11

f1x: parameterize toError, fromError and replacer

wip: tofrom

fix: parameterize toError, fromError and replacer

fix: Makes concurrent jests non deterministic

* Related #4

fix: parameterize replacer toError and fromError, change fromError to return JSONValue, stringify fromError usages

* Related #10

fix: Converted global state for fromError to handle it internally.

*Related: #10
Reviewed-by: @tegefaulkes
[ci skip]

chore: Jests for fromError and toError, and using a custom replacer.

related: #10

[ci skip]
addievo pushed a commit that referenced this issue Oct 9, 2023
@addievo
# This is a combination of 16 commits.
63e13af 
# This is the 1st commit message:

fix: RPCServer.start is now no longer a factory function

fix: fixed RPC tests after async-init change

# This is the commit message #2:

chore: updated inline documentation according to async-init changes

# This is the commit message #3:

wip: fixing imports

# This is the commit message #4:

wip: RPCServer deconstruction

# This is the commit message #5:

fix: RPCServer start stop order

# This is the commit message #6:

fix: added back createDestroy back to RPCClient

# This is the commit message #7:

chore: lintfix

# This is the commit message #8:

wip: completely removed create destroy from rpcclient

# This is the commit message #9:

fix: RPCClient tests after async-init changes

# This is the commit message #10:

wip

# This is the commit message #11:

fix: idgen is optional in constructor

# This is the commit message #12:

fix: type import in ./types

# This is the commit message #13:

chore: test for RPCServer force stopping

# This is the commit message #14:

fix: proper implementation of fromError toError, clientOutputTransformStream no longer outputs error.data, but rather error directly

# This is the commit message #15:

fix: jest fix for ErrorRPCRemote

# This is the commit message #16:

wip toError fromError

fix: proper implementation of fromError toError, clientOutputTransformStream no longer outputs error.data, but rather error directly

fix: changed rpcClient toError implementation

fix: changing ErrorRPCRemote constructor to be in-line with toError error creation constructor

fix: jest fix for ErrorRPCRemote

fix: fixing exports

fix: RPC errors now correctly extend AbstractError

fix: removed old events
fix: cleaned up imports
feat: client has toError as parameter
fix: removed type from JSONError obj
fix: startStop constructor correctly used
fix: infinity is definitely not == 1 minute

chore:  rename variable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tegefaulkes tegefaulkes

Labels
development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Factor out RPC from Polykey MatrixAI/js-rpc
3 participants
@CMCDragonkai @tegefaulkes @addievo