Skip to content

Commit

Permalink
feat!: update ucanto to invocation spec compatible result type (#272)
Browse files Browse the repository at this point in the history
- Updates result type to use ucan invocation spec compatible representation
- At the library layer (except of the schemas which needs to deal with optional fields) we now enforce no `null|undefined` values in Results and push towards use of unit value `{}` instead.
   - Mostly this guides users into a better and more extensible API design.
   - Please note that `false` and `""` are still allowed.
- ⚠️ above changes caused breaking API change on `.derives` methods than no longer return `true` or `Failure` but instead return `Result<{}, Failure>`.
- ⚠️ Error types no longer have `error: true` field which were in place to pattern match on ok / error.
  • Loading branch information
Gozala authored Apr 4, 2023
1 parent aeea7e3 commit b124ed8
Show file tree
Hide file tree
Showing 44 changed files with 1,275 additions and 942 deletions.
15 changes: 4 additions & 11 deletions packages/client/test/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ import * as API from '@ucanto/interface'

import type { DID, Link, Await, Result as SyncResult } from '@ucanto/interface'
export type { DID, Link, SyncResult }
type Result<
T extends unknown = unknown,
X extends { error: true } = Error & { error: true }
> = Await<API.Result<T, X>>
type Result<T extends {} = {}, X extends {} = Error> = Await<API.Result<T, X>>

export interface StorageProvider {
/**
Expand Down Expand Up @@ -34,20 +31,20 @@ export interface StorageProvider {
group: DID,
link: Link,
proof: Link
): Result<null, UnknownDIDError | DoesNotHasError>
): Result<{}, UnknownDIDError | DoesNotHasError>
}

export interface TokenStore {
/**
* Revokes set of UCANS. CID corresponds to the link been revoked and
* proof is the CID of the revocation.
*/
revoke(token: Link, revocation: TokenEntry): Result<null, RevokeError>
revoke(token: Link, revocation: TokenEntry): Result<{}, RevokeError>

/**
* Adds bunch of proofs for later queries.
*/
insert(tokens: IterableIterator<TokenEntry>): Result<null, InsertError>
insert(tokens: IterableIterator<TokenEntry>): Result<{}, InsertError>

/**
* You give it named set of CIDs and it gives you back named set of
Expand Down Expand Up @@ -185,13 +182,9 @@ export interface DoesNotHasError extends RangeError {
export interface UnknownDIDError extends RangeError {
readonly name: 'UnknownDIDError'
did: DID | null

error: true
}

export interface InvalidInvocation extends Error {
readonly name: 'InvalidInvocation'
link: Link

error: true
}
6 changes: 2 additions & 4 deletions packages/client/test/client.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ const channel = HTTP.open({
return Receipt.issue({
ran: invocation.cid,
issuer: w3,
result: result?.error ? { error: result } : { ok: result },
result,
})
}
case 'store/remove': {
Expand All @@ -162,7 +162,7 @@ const channel = HTTP.open({
return Receipt.issue({
ran: invocation.cid,
issuer: w3,
result: result?.error ? { error: result } : { ok: result },
result,
})
}
}
Expand Down Expand Up @@ -219,7 +219,6 @@ test('execute', async () => {

assert.deepEqual(e1.out, {
error: {
error: true,
name: 'UnknownDIDError',
message: `DID ${alice.did()} has no account`,
did: alice.did(),
Expand Down Expand Up @@ -264,7 +263,6 @@ test('execute with delegations', async () => {

assert.deepEqual(e1.out, {
error: {
error: true,
name: 'UnknownDIDError',
message: `DID ${bob.did()} has no account`,
did: bob.did(),
Expand Down
24 changes: 14 additions & 10 deletions packages/client/test/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,22 @@ class StorageService {
/** @type {any} */ (ucan).cid
)
if (!result.error) {
if (result.status === 'in-s3') {
if (result.ok.status === 'in-s3') {
return {
with: capability.with,
link: capability.nb.link,
status: the('done'),
ok: {
with: capability.with,
link: capability.nb.link,
status: the('done'),
},
}
} else {
return {
with: capability.with,
link: capability.nb.link,
status: the('upload'),
url: 'http://localhost:9090/',
ok: {
with: capability.with,
link: capability.nb.link,
status: the('upload'),
url: 'http://localhost:9090/',
},
}
}
} else {
Expand All @@ -93,10 +97,10 @@ class StorageService {
capability.nb.link,
/** @type {any} */ (ucan).link
)
if (remove?.error) {
if (remove.error) {
return remove
} else {
return capability
return { ok: capability }
}
}
}
Expand Down
12 changes: 4 additions & 8 deletions packages/client/test/services/account.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ const unlink = (model, member, group, proof) => {
if (account === resolve(model, member)) {
model.delete(member)
}
return {}
return { ok: {} }
} else {
return new UnknownDIDError('Unknown DID', group)
return { error: new UnknownDIDError('Unknown DID', group) }
}
}

Expand All @@ -98,7 +98,7 @@ const associate = (accounts, from, to, proof, create) => {
accounts.set(to, { account, proof })
accounts.set(from, { account, proof })
} else {
return new UnknownDIDError('Unknown did', to)
return { error: new UnknownDIDError('Unknown did', to) }
}
} else if (toAccount) {
accounts.set(from, { account: toAccount, proof })
Expand All @@ -110,7 +110,7 @@ const associate = (accounts, from, to, proof, create) => {
accounts.set(fromAccount, { account, proof })
}

return {}
return { ok: {} }
}

/**
Expand Down Expand Up @@ -147,9 +147,6 @@ export class UnknownDIDError extends RangeError {
super(message)
this.did = did
}
get error() {
return /** @type {true} */ (true)
}
get name() {
return the('UnknownDIDError')
}
Expand All @@ -159,7 +156,6 @@ export class UnknownDIDError extends RangeError {
name: this.name,
message: this.message,
did: this.did,
error: true,
}
}
}
14 changes: 8 additions & 6 deletions packages/client/test/services/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { the } from './util.js'
* @param {Partial<Model> & { accounts: API.AccessProvider }} config
* @returns {API.StorageProvider}
*/
export const create = (config) => new StoreProvider(config)
export const create = config => new StoreProvider(config)

/**
* @typedef {{
Expand Down Expand Up @@ -43,9 +43,11 @@ export const add = async ({ accounts, groups, cars }, group, link, proof) => {
const links = groups.get(group) || new Map()
links.set(`${link}`, link)
groups.set(group, links)
return { status: cars.get(`${link}`) ? the('in-s3') : the('not-in-s3') }
return {
ok: { status: cars.get(`${link}`) ? the('in-s3') : the('not-in-s3') },
}
} else {
return new UnknownDIDError(`DID ${group} has no account`, group)
return { error: new UnknownDIDError(`DID ${group} has no account`, group) }
}
}

Expand All @@ -60,10 +62,10 @@ export const remove = async ({ accounts, groups }, group, link, proof) => {
if (account) {
const links = groups.get(group)
return links && links.get(`${link}`)
? null
: new DoesNotHasError(group, link)
? { ok: {} }
: { error: new DoesNotHasError(group, link) }
} else {
return new UnknownDIDError(`DID ${group} has no account`, group)
return { error: new UnknownDIDError(`DID ${group} has no account`, group) }
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/client/test/services/tokens.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { the, unreachable } from './util.js'
* @param {Map<string, API.Found|API.RevokedError|API.ExpiredError>} store
* @returns {API.TokenStore}
*/
export const create = (store) => new TokenService(store)
export const create = store => new TokenService(store)

/**
* @implements {API.TokenStore}
Expand Down Expand Up @@ -41,7 +41,7 @@ class TokenService {
return unreachable`Record has unexpected state ${record}`
}
}
return null
return { ok: {} }
}
/**
* @template {Record<string, API.Link>} Query
Expand Down Expand Up @@ -80,7 +80,7 @@ class TokenService {
return unreachable`record has unknown state ${record}`
}

return null
return { ok: {} }
}

async gc() {
Expand Down
4 changes: 4 additions & 0 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@
"./dag": {
"types": "./dist/src/dag.d.ts",
"import": "./src/dag.js"
},
"./result": {
"types": "./dist/src/result.d.ts",
"import": "./src/result.js"
}
},
"c8": {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/lib.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ export { sha256 } from 'multiformats/hashes/sha2'
export * as UCAN from '@ipld/dag-ucan'
export * as DID from '@ipld/dag-ucan/did'
export * as Signature from '@ipld/dag-ucan/signature'
export * from './result.js'
4 changes: 2 additions & 2 deletions packages/core/src/receipt.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ class ReceptBuilder {
* @param {object} options
* @param {API.Signer<API.DID, SigAlg>} options.issuer
* @param {Ran|ReturnType<Ran['link']>} options.ran
* @param {API.ReceiptResult<Ok, Error>} options.result
* @param {API.Result<Ok, Error>} options.result
* @param {API.EffectsModel} [options.fx]
* @param {API.Proof[]} [options.proofs]
* @param {Record<string, unknown>} [options.meta]
Expand Down Expand Up @@ -252,7 +252,7 @@ const NOFX = Object.freeze({ fork: Object.freeze([]) })
* @param {object} options
* @param {API.Signer<API.DID, SigAlg>} options.issuer
* @param {Ran|ReturnType<Ran['link']>} options.ran
* @param {API.ReceiptResult<Ok, Error>} options.result
* @param {API.Result<Ok, Error>} options.result
* @param {API.EffectsModel} [options.fx]
* @param {API.Proof[]} [options.proofs]
* @param {Record<string, unknown>} [options.meta]
Expand Down
63 changes: 63 additions & 0 deletions packages/core/src/result.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import * as API from '@ucanto/interface'

/**
* Creates the success result containing given `value`. Throws if
* `null` or `undefined` passed to encourage use of units instead.
*
* @template {{}|string|boolean|number} T
* @param {T} value
* @returns {{ok: T, value?:undefined}}
*/
export const ok = value => {
if (value == null) {
throw new TypeError(`ok(${value}) is not allowed, consider ok({}) instead`)
} else {
return { ok: value }
}
}

/**
* Creates the failing result containing given `cause` of error.
* Throws if `cause` is `null` or `undefined` to encourage
* passing descriptive errors instead.
*
* @template {{}|string|boolean|number} X
* @param {X} cause
* @returns {{ok?:undefined, error:X}}
*/
export const error = cause => {
if (cause == null) {
throw new TypeError(
`error(${cause}) is not allowed, consider passing an error instead`
)
} else {
return { error: cause }
}
}

/**
* Creates the failing result containing an error with a given
* `message`. Unlike `error` function it creates a very generic
* error with `message` & `stack` fields. The `error` function
* is recommended over `fail` for all but the most basic use cases.
*
* @param {string} message
* @returns {{error:API.Failure, ok?:undefined}}
*/
export const fail = message => ({ error: new Failure(message) })

/**
* @implements {API.Failure}
*/
export class Failure extends Error {
describe() {
return this.toString()
}
get message() {
return this.describe()
}
toJSON() {
const { name, message, stack } = this
return { name, message, stack }
}
}
Loading

0 comments on commit b124ed8

Please sign in to comment.