This repository was archived by the owner on Jul 10, 2025. It is now read-only.
Update JS SDK api to the new version #61
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ly refers to the gitbook page
coder11
force-pushed
the
new-js-sdk-api
branch
from
15f79dd to
174477e
Compare
This reverts commit a8a6a81.
coder11
force-pushed
the
new-js-sdk-api
branch
from
f1871ff to
b09ef7d
Compare
coder11
force-pushed
the
new-js-sdk-api
branch
from
037ba3b to
480d630
Compare
This reverts commit 96f8a0a.
folex
reviewed
Sep 7, 2021
src/internal/FluencePeer.ts
Outdated
| * * Multiaddr - multiaddr object, @see https://github.com/multiformats/js-multiaddr | ||
| * * Node - node structure, @see Node | ||
| */ | ||
| export type ConnectionSpec = string | MA | Node; |
There was a problem hiding this comment.
Specification. Feel free to suggest a better name
There was a problem hiding this comment.
Remote / ConnectionTarget / Node / Relay / Peer
There was a problem hiding this comment.
I've just got rid of the explicit type definition
src/internal/FluencePeer.ts
Outdated
| @@ -0,0 +1,350 @@ | |||
| import { AirInterpreter, CallServiceResult, LogLevel, ParticleHandler, SecurityTetraplet } from '@fluencelabs/avm'; | |||
| import log from 'loglevel'; | |||
| import { Multiaddr as MA } from 'multiaddr'; | |||
There was a problem hiding this comment.
Let's keep Multiaddress, MA is cryptic
src/internal/FluencePeer.ts
Outdated
| peerId = peerIdOrSeed; | ||
| } else { | ||
| // peerId is string, therefore seed | ||
| peerId = await peerIdFromEd25519SK(peerIdOrSeed); |
There was a problem hiding this comment.
What will happen if peerIdOrSeed is malformed?
There was a problem hiding this comment.
We decided to replace this logic with the help of KeyPair class
src/internal/FluencePeer.ts
Outdated
| const request = this._requests.get(this._currentRequestId); | ||
| const particle = request.getParticle(); | ||
| if (particle === null) { | ||
| throw new Error("particle can't be null here"); |
There was a problem hiding this comment.
Let's add some debug info to the error? At least this._currentRequestId
src/internal/FluencePeer.ts
Outdated
|
|
||
| if (res.result === undefined) { | ||
| log.error( | ||
| `Call to serviceId=${serviceId} fnName=${fnName} unexpectedly returned undefined result, falling back to null`, |
src/internal/FluencePeer.ts
Outdated
| this._requests.delete(key); | ||
| } | ||
| } | ||
| }, 5000); |
There was a problem hiding this comment.
Let's add todo to make 5000 configurable
src/internal/RequestFlow.ts
Outdated
| @@ -149,6 +150,7 @@ relay peer id: ${this.relayPeerId} | |||
| // TODO:: keep the history of particle data mb? | |||
| this.prevData = this.state.data; | |||
| this.state.data = particle.data; | |||
| log.debug('Received update'); | |||
src/internal/defaultMiddlewares.ts
Outdated
| next(); | ||
| } catch (e) { | ||
| resp.retCode = ResultCodes.exceptionInHandler; | ||
| resp.result = e.toString(); |
There was a problem hiding this comment.
Let's add more debug info to the error? Particle id maybe? service id and fnName at least.
coder11
force-pushed
the
new-js-sdk-api
branch
from
90e42f2 to
72bdea4
Compare
folex
approved these changes
Sep 8, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first step of the massive JS SDK refacroing. Currently it only migrates to the new api format without major underlying changes.
Brief overview of what was changed: