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

feat!: implement receipts #266

Merged
merged 14 commits into from
Mar 30, 2023
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ node_modules
coverage
.pnpm-debug.log
.env
.vscode
6 changes: 0 additions & 6 deletions .vscode/settings.json

This file was deleted.

3 changes: 1 addition & 2 deletions packages/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,11 @@
},
"dependencies": {
"@ucanto/interface": "workspace:^",
"multiformats": "^11.0.0"
"@ucanto/core": "workspace:^"
},
"devDependencies": {
"@types/chai": "^4.3.3",
"@types/mocha": "^10.0.1",
"@ucanto/core": "workspace:^",
"@ucanto/principal": "workspace:^",
"@ucanto/transport": "workspace:^",
"@web-std/fetch": "^4.1.0",
Expand Down
51 changes: 41 additions & 10 deletions packages/client/src/connection.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as API from '@ucanto/interface'
import { sha256 } from 'multiformats/hashes/sha2'
import { Receipt, Signature, sha256 } from '@ucanto/core'

/**
* Creates a connection to a service.
Expand All @@ -8,7 +8,7 @@ import { sha256 } from 'multiformats/hashes/sha2'
* @param {API.ConnectionOptions<T>} options
* @returns {API.ConnectionView<T>}
*/
export const connect = (options) => new Connection(options)
export const connect = options => new Connection(options)

/**
* @template {Record<string, any>} T
Expand All @@ -21,8 +21,7 @@ class Connection {
constructor(options) {
this.id = options.id
this.options = options
this.encoder = options.encoder
this.decoder = options.decoder
this.codec = options.codec
this.channel = options.channel
this.hasher = options.hasher || sha256
}
Expand All @@ -41,12 +40,44 @@ class Connection {
* @template {Record<string, any>} T
* @template {API.Tuple<API.ServiceInvocation<C, T>>} I
* @param {API.Connection<T>} connection
* @param {I} invocations
* @returns {Promise<API.InferServiceInvocations<I, T>>}
* @param {I} workflow
* @returns {Promise<API.InferWorkflowReceipts<I, T>>}
*/
export const execute = async (invocations, connection) => {
const request = await connection.encoder.encode(invocations, connection)
export const execute = async (workflow, connection) => {
const request = await connection.codec.encode(workflow, connection)
const response = await connection.channel.request(request)
const result = await connection.decoder.decode(response)
return result
// We may fail to decode the response if content type is not supported
// or if data was corrupted. We do not want to throw in such case however,
// because client will get an Error object as opposed to a receipt, to retain
// consistent client API with two kinds of errors we encode caught error as
// a receipts per workflow invocation.
try {
return await connection.codec.decode(response)
} catch (error) {
const { message, ...cause } = /** @type {Error} */ (error)
Gozala marked this conversation as resolved.
Show resolved Hide resolved
const receipts = []
for await (const invocation of workflow) {
const { cid } = await invocation.delegate()
const receipt = await Receipt.issue({
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels weird that an error in the channel + codec working together would lead to a Receipt vs just not catching the error here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would be happy for it to just throw/reject when encountering unexpected errors here vs 'wrapping' something it doesn't understand (error is unknown).
In doing so, I think this takes the thrown object, throws away all properties other than 'message' and 'cause'. It would be frustrating if those properties were important to debug some mistake.

Copy link
Contributor

Choose a reason for hiding this comment

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

(i.e. if we wrap at all, I think we should only wrap errors from successful decodes, not errors encountered while decoding, i.e. errors where the error might have been in the decoder vs what was on the other side of the channel)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree this is not ideal, yet I feel very strongly that two channels for error handling is going to be a super error prone.

try {
   const receipt = await agent.invoke(capbility)
    if (receipt.out.error) {
         // handle error
    }
} catch (error) {
   // handle error
}

Perhaps we could come up with a better solution so that they are separate steps, but lets do it in a followup

ran: cid,
result: { error: { ...cause, message } },
// @ts-expect-error - we can not really sign a receipt without having
// an access to a signer which client does not have. In the future
// we will change client API requiring a signer to be passed in but
// for now we just use a dummy signer.
issuer: {
did() {
return connection.id.did()
},
sign() {
return Signature.createNonStandard('', new Uint8Array())
},
},
})

receipts.push(receipt)
}

return /** @type {any} */ (receipts)
}
}
6 changes: 3 additions & 3 deletions packages/client/test/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,16 +133,16 @@ export interface AccessProvider {
* Associates a DID with another DID in the system. If there is no account
* associated with a `to` DID will produce an error.
*/
link(member: DID, group: DID, proof: Link): Result<null, UnknownDIDError>
link(member: DID, group: DID, proof: Link): Result<{}, UnknownDIDError>

unlink(member: DID, group: DID, proof: Link): Result<null, UnknownDIDError>
unlink(member: DID, group: DID, proof: Link): Result<{}, UnknownDIDError>

/**
* Associates new child DID with an accound of the parent DID. If there is no
* account associated with a parent it creates account with `parent` did first
* and then associates child DID with it.
*/
register(member: DID, group: DID, proof: Link): Result<null, UnknownDIDError>
register(member: DID, group: DID, proof: Link): Result<{}, UnknownDIDError>

/**
* Resolves account DID associated with a given DID. Returns either account
Expand Down
Loading