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

fix: error interactions regression #293

Merged
merged 3 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions examples/simple-dapp/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ resetButton.onclick = () => {
window.location.replace(window.location.origin)
}

sendTxButton.onclick = () => {
dAppToolkit.walletApi.sendTransaction({
sendTxButton.onclick = async () => {
const res = await dAppToolkit.walletApi.sendTransaction({
transactionManifest: `
CALL_METHOD
Address("component_tdx_2_1cptxxxxxxxxxfaucetxxxxxxxxx000527798379xxxxxxxxxyulkzl")
Expand All @@ -232,6 +232,7 @@ sendTxButton.onclick = () => {
Enum<0u8>()
;`,
})
console.log('send tx result', res)
}

oneTimeRequest.onclick = () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/dapp-toolkit/src/_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
Persona,
PersonaDataName,
WalletInteraction,
WalletInteractionResponse,
} from './schemas'
import type { Logger } from './helpers'
import type { SdkError } from './error'
Expand Down Expand Up @@ -175,7 +176,7 @@ export type TransportProvider = {
send: (
walletInteraction: WalletInteraction,
callbackFns: Partial<CallbackFns>,
) => ResultAsync<unknown, SdkError>
) => ResultAsync<WalletInteractionResponse, SdkError>
disconnect: () => void
destroy: () => void
}
Expand Down
31 changes: 8 additions & 23 deletions packages/dapp-toolkit/src/helpers/validate-wallet-response.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { describe, it, expect } from 'vitest'
import { validateWalletResponse } from './validate-wallet-response'

describe('validateWalletResponse', () => {
it('should parse valid response', async () => {
it('should parse valid response', () => {
const walletResponse = {
discriminator: 'success',
interactionId: 'ab0f0190-1ae1-424b-a2c5-c36838f5b136',
Expand Down Expand Up @@ -38,7 +38,7 @@ describe('validateWalletResponse', () => {
},
}

const result = await validateWalletResponse(walletResponse)
const result = validateWalletResponse(walletResponse)

expect(result.isOk() && result.value).toEqual(walletResponse)
})
Expand All @@ -48,26 +48,11 @@ describe('validateWalletResponse', () => {

const result = await validateWalletResponse(walletResponse)

expect(result.isErr() && result.error).toEqual({
error: 'walletResponseValidation',
interactionId: '',
message: 'Invalid input',
})
})

it('should return error valid failure response', async () => {
const walletResponse = {
discriminator: 'failure',
interactionId: '8cefec84-542d-40af-8782-b89df05db8ac',
error: 'rejectedByUser',
}

const result = await validateWalletResponse(walletResponse)

expect(result.isErr() && result.error).toEqual({
discriminator: 'failure',
interactionId: '8cefec84-542d-40af-8782-b89df05db8ac',
error: 'rejectedByUser',
})
expect(result.isErr() && result.error).toEqual(
expect.objectContaining({
error: 'walletResponseValidation',
message: 'Invalid input',
}),
)
})
})
30 changes: 11 additions & 19 deletions packages/dapp-toolkit/src/helpers/validate-wallet-response.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,22 @@
import { Result, ResultAsync, errAsync, okAsync } from 'neverthrow'
import {
WalletInteractionResponse,
WalletInteractionSuccessResponse,
} from '../schemas'
import { Result } from 'neverthrow'
import { WalletInteractionResponse } from '../schemas'
import { SdkError } from '../error'
import { parse } from 'valibot'

export const validateWalletResponse = (
walletResponse: unknown,
): ResultAsync<
WalletInteractionSuccessResponse,
SdkError | { discriminator: 'failure'; interactionId: string; error: string }
> => {
): Result<WalletInteractionResponse, SdkError> => {
const fn = Result.fromThrowable(
(_) => parse(WalletInteractionResponse, _),
(error) => error,
)

const result = fn(walletResponse)
if (result.isErr()) {
return errAsync(SdkError('walletResponseValidation', '', 'Invalid input'))
} else if (result.isOk()) {
return result.value.discriminator === 'success'
? okAsync(result.value)
: errAsync(result.value)
}

return errAsync(SdkError('walletResponseValidation', ''))
return fn(walletResponse).mapErr((response) =>
SdkError(
'walletResponseValidation',
(walletResponse as any)?.interactionId,
'Invalid input',
response,
),
)
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { err, ok, okAsync, ResultAsync } from 'neverthrow'
import { err, ok, okAsync, Result, ResultAsync } from 'neverthrow'
import { validateWalletResponse, type Logger } from '../../../helpers'
import type { WalletInteractionResponse } from '../../../schemas'
import type { StorageModule } from '../../storage'
Expand Down Expand Up @@ -62,17 +62,19 @@ export const RequestResolverModule = (input: {
requestItemModule.patch(interactionId, { sentToWallet: true })

const addWalletResponses = (responses: WalletInteractionResponse[]) =>
ResultAsync.combine(responses.map(validateWalletResponse)).andThen(() =>
walletResponses.setItems(
responses.reduce<Record<string, WalletInteractionResponse>>(
(acc, response) => {
acc[response.interactionId] = response
return acc
},
{},
Result.combine(responses.map(validateWalletResponse))
.asyncAndThen(() =>
walletResponses.setItems(
responses.reduce<Record<string, WalletInteractionResponse>>(
(acc, response) => {
acc[response.interactionId] = response
return acc
},
{},
),
),
),
)
)
.mapErr((error) => logger?.error({ method: 'addWalletResponses', error }))
Copy link
Collaborator

Choose a reason for hiding this comment

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

returns the logger object ILogObjMeta

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah saw that decided to not change as it's not a problem, this error is not consumed anywhere after that. I can make it return undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or map to SdkError just for the sake of consistency?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not consumed. Does not really matter


const toRequestItemMap = (items: RequestItem[]) =>
items.reduce<Record<string, RequestItem>>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
MessageLifeCycleExtensionStatusEvent,
WalletInteraction,
WalletInteractionExtensionInteraction,
WalletInteractionResponse,
eventType,
} from '../../../../schemas'
import { StorageModule } from '../../../storage'
Expand Down Expand Up @@ -143,7 +144,7 @@ export const ConnectorExtensionModule = (input: {
const sendWalletInteraction = (
walletInteraction: WalletInteraction,
callbackFns: Partial<CallbackFns>,
): ResultAsync<unknown, SdkError> => {
): ResultAsync<WalletInteractionResponse, SdkError> => {
const cancelRequestSubject = new Subject<Err<never, SdkError>>()

const maybeResolved$ = from(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export const RadixConnectRelayModule = (input: {
const sendToWallet = (
walletInteraction: WalletInteraction,
callbackFns: Partial<CallbackFns>,
): ResultAsync<unknown, SdkError> =>
): ResultAsync<WalletInteractionResponse, SdkError> =>
ResultAsync.combine([
sessionModule
.getCurrentSession()
Expand Down Expand Up @@ -194,6 +194,7 @@ export const RadixConnectRelayModule = (input: {
walletInteraction.interactionId,
),
)
.map((requestItem) => requestItem.walletResponse),
)

const decryptWalletResponseData = (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { okAsync, ResultAsync } from 'neverthrow'
import { TransportProvider } from '../../../../_types'
import { WalletInteractionResponse } from '../../../../schemas'

export const TestingTransportModule = ({
requestResolverModule,
}: {
requestResolverModule: {
addWalletResponses: (
responses: WalletInteractionResponse[],
) => ResultAsync<unknown, unknown>
}
}): TransportProvider & {
setNextWalletResponse: (response: WalletInteractionResponse) => void
} => {
let _isSupported = true
let id = 'TestingTransportModule'
let _sendResponse: WalletInteractionResponse

return {
id,
isSupported: () => _isSupported,
destroy: () => {},
disconnect: () => {},
send: () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not use vitest fn for this purpose?

requestResolverModule.addWalletResponses([_sendResponse])
return okAsync(_sendResponse)
},

// Test helpers
setNextWalletResponse: (response: WalletInteractionResponse) => {
_sendResponse = response
},
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { Result, ResultAsync, err, ok } from 'neverthrow'
import { Result, ResultAsync, err, errAsync, ok, okAsync } from 'neverthrow'
import { TransportProvider } from '../../_types'
import { Logger, validateWalletResponse } from '../../helpers'
import { Logger } from '../../helpers'
import {
Metadata,
CallbackFns,
WalletInteractionItems,
WalletInteraction,
WalletInteractionFailureResponse,
WalletInteractionResponse,
WalletInteractionSuccessResponse,
} from '../../schemas'
import { parse } from 'valibot'
import { SdkError } from '../../error'
Expand All @@ -22,6 +24,7 @@ export type WalletRequestSdkInput = {
) => Promise<WalletInteraction>
providers: {
transports: TransportProvider[]
interactionIdFactory?: () => string
}
}
export type WalletRequestSdk = ReturnType<typeof WalletRequestSdk>
Expand All @@ -34,6 +37,8 @@ export const WalletRequestSdk = (input: WalletRequestSdkInput) => {
origin: input.origin || window.location.origin,
} as Metadata

const interactionIdFactory = input.providers.interactionIdFactory ?? uuidV4

parse(Metadata, metadata)

const logger = input?.logger?.getSubLogger({ name: 'WalletSdk' })
Expand All @@ -50,7 +55,7 @@ export const WalletRequestSdk = (input: WalletRequestSdkInput) => {

const createWalletInteraction = (
items: WalletInteractionItems,
interactionId = uuidV4(),
interactionId = interactionIdFactory(),
): WalletInteraction => ({
items,
interactionId,
Expand Down Expand Up @@ -86,15 +91,24 @@ export const WalletRequestSdk = (input: WalletRequestSdkInput) => {
items,
}: { interactionId?: string; items: WalletInteraction['items'] },
callbackFns: Partial<CallbackFns> = {},
): ResultAsync<unknown, SdkError> =>
): ResultAsync<
WalletInteractionSuccessResponse,
SdkError | WalletInteractionFailureResponse
Copy link
Collaborator

@xstelea xstelea Dec 6, 2024

Choose a reason for hiding this comment

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

can we keep it as SdkError only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No because it'd not be adequate. It can error with FailureResponse that's actually the whole point of this regression :)

> =>
withInterceptor({
items,
interactionId,
metadata,
}).andThen((walletInteraction) =>
getTransport(walletInteraction.interactionId).asyncAndThen((transport) =>
transport.send(walletInteraction, callbackFns),
),
getTransport(walletInteraction.interactionId)
.asyncAndThen((transport) =>
transport.send(walletInteraction, callbackFns),
)
.andThen((response) =>
response.discriminator === 'failure'
? errAsync(response)
: okAsync(response),
),
)

return {
Expand Down
Loading
Loading