fix: load screen on sign, block on missing counterpart #170

Merged
enes merged 7 commits from 137-sign-loading-error into staging 2024-08-29 11:02:19 +00:00
8 changed files with 219 additions and 209 deletions

View File

@ -11,7 +11,6 @@ import {
hexToNpub, hexToNpub,
parseNostrEvent, parseNostrEvent,
parseCreateSignatureEventContent, parseCreateSignatureEventContent,
SigitMetaParseError,
SigitStatus, SigitStatus,
SignStatus SignStatus
} from '../utils' } from '../utils'
@ -21,6 +20,7 @@ import { Event } from 'nostr-tools'
import store from '../store/store' import store from '../store/store'
import { AuthState } from '../store/auth/types' import { AuthState } from '../store/auth/types'
import { NostrController } from '../controllers' import { NostrController } from '../controllers'
import { MetaParseError } from '../types/errors/MetaParseError'
/** /**
* Flattened interface that combines properties `Meta`, `CreateSignatureEventContent`, * Flattened interface that combines properties `Meta`, `CreateSignatureEventContent`,
@ -247,7 +247,7 @@ export const useSigitMeta = (meta: Meta): FlatMeta => {
) )
} }
} catch (error) { } catch (error) {
if (error instanceof SigitMetaParseError) { if (error instanceof MetaParseError) {
toast.error(error.message) toast.error(error.message)
} }
console.error(error) console.error(error)

View File

@ -17,8 +17,10 @@ export const StickySideColumns = ({
<div className={`${styles.sidesWrap} ${styles.files}`}> <div className={`${styles.sidesWrap} ${styles.files}`}>
<div className={styles.sides}>{left}</div> <div className={styles.sides}>{left}</div>
</div> </div>
<div id="content-preview" className={styles.content}> <div>
{children} <div id="content-preview" className={styles.content}>
{children}
</div>
</div> </div>
<div className={styles.sidesWrap}> <div className={styles.sidesWrap}>
<div className={styles.sides}>{right}</div> <div className={styles.sides}>{right}</div>

View File

@ -514,6 +514,9 @@ export const CreatePage = () => {
return ( return (
file.pages?.flatMap((page, index) => { file.pages?.flatMap((page, index) => {
return page.drawnFields.map((drawnField) => { return page.drawnFields.map((drawnField) => {
if (!drawnField.counterpart) {
throw new Error('Missing counterpart')
}
return { return {
type: drawnField.type, type: drawnField.type,
location: { location: {
@ -670,6 +673,7 @@ export const CreatePage = () => {
} }
const generateCreateSignature = async ( const generateCreateSignature = async (
markConfig: Mark[],
fileHashes: { fileHashes: {
[key: string]: string [key: string]: string
}, },
@ -677,7 +681,6 @@ export const CreatePage = () => {
) => { ) => {
const signers = users.filter((user) => user.role === UserRole.signer) const signers = users.filter((user) => user.role === UserRole.signer)
const viewers = users.filter((user) => user.role === UserRole.viewer) const viewers = users.filter((user) => user.role === UserRole.viewer)
const markConfig = createMarks(fileHashes)
const content: CreateSignatureEventContent = { const content: CreateSignatureEventContent = {
signers: signers.map((signer) => hexToNpub(signer.pubkey)), signers: signers.map((signer) => hexToNpub(signer.pubkey)),
@ -721,131 +724,128 @@ export const CreatePage = () => {
} }
const handleCreate = async () => { const handleCreate = async () => {
if (!validateInputs()) return try {
if (!validateInputs()) return
setIsLoading(true) setIsLoading(true)
Review

I wonder if this flow should be refactored, given our recent conversation about error handling. I also thing that there are more efficient ways to handle loading state changes.

Having said that, perhaps it is best left to a separate PR. Worth adding as an issue?

I wonder if this flow should be refactored, given our recent conversation about error handling. I also thing that there are more efficient ways to handle loading state changes. Having said that, perhaps it is best left to a separate PR. Worth adding as an issue?
Review

I think we can for now just leave finally, to execute setIsLoading(false) once, we don't need it in each return

I think we can for now just leave `finally`, to execute `setIsLoading(false)` once, we don't need it in each return
setLoadingSpinnerDesc('Generating file hashes') setLoadingSpinnerDesc('Generating file hashes')
const fileHashes = await generateFileHashes() const fileHashes = await generateFileHashes()
if (!fileHashes) { if (!fileHashes) return
setLoadingSpinnerDesc('Generating encryption key')
const encryptionKey = await generateEncryptionKey()
if (await isOnline()) {
setLoadingSpinnerDesc('generating files.zip')
const arrayBuffer = await generateFilesZip()
if (!arrayBuffer) return
setLoadingSpinnerDesc('Encrypting files.zip')
const encryptedArrayBuffer = await encryptZipFile(
arrayBuffer,
encryptionKey
)
const markConfig = createMarks(fileHashes)
setLoadingSpinnerDesc('Uploading files.zip to file storage')
const fileUrl = await uploadFile(encryptedArrayBuffer)
if (!fileUrl) return
setLoadingSpinnerDesc('Generating create signature')
const createSignature = await generateCreateSignature(
markConfig,
fileHashes,
fileUrl
)
if (!createSignature) return
setLoadingSpinnerDesc('Generating keys for decryption')
// generate key pairs for decryption
const pubkeys = users.map((user) => user.pubkey)
// also add creator in the list
if (pubkeys.includes(usersPubkey!)) {
pubkeys.push(usersPubkey!)
}
const keys = await generateKeys(pubkeys, encryptionKey)
if (!keys) return
const meta: Meta = {
createSignature,
keys,
modifiedAt: unixNow(),
docSignatures: {}
}
setLoadingSpinnerDesc('Updating user app data')
const event = await updateUsersAppData(meta)
if (!event) return
setLoadingSpinnerDesc('Sending notifications to counterparties')
const promises = sendNotifications(meta)
await Promise.all(promises)
.then(() => {
toast.success('Notifications sent successfully')
})
.catch(() => {
toast.error('Failed to publish notifications')
})
navigate(appPrivateRoutes.sign, { state: { meta: meta } })
} else {
const zip = new JSZip()
selectedFiles.forEach((file) => {
zip.file(`files/${file.name}`, file)
})
const markConfig = createMarks(fileHashes)
setLoadingSpinnerDesc('Generating create signature')
Review

A promise doesn't need to be awaited if chaining is used.

A promise doesn't need to be awaited if chaining is used.
Review

We are blocking so we don't navigate away before .all is done.
Another question is if we need to block at all since even if it's rejected, we proceed to another page anyway?

We are blocking so we don't navigate away before `.all` is done. Another question is if we need to block at all since even if it's rejected, we proceed to another page anyway?
const createSignature = await generateCreateSignature(
markConfig,
fileHashes,
''
)
if (!createSignature) return
const meta: Meta = {
createSignature,
modifiedAt: unixNow(),
docSignatures: {}
}
// add meta to zip
try {
const stringifiedMeta = JSON.stringify(meta, null, 2)
zip.file('meta.json', stringifiedMeta)
} catch (err) {
console.error(err)
toast.error('An error occurred in converting meta json to string')
return null
}
const arrayBuffer = await generateZipFile(zip)
if (!arrayBuffer) return
setLoadingSpinnerDesc('Encrypting zip file')
const encryptedArrayBuffer = await encryptZipFile(
arrayBuffer,
encryptionKey
)
await handleOfflineFlow(encryptedArrayBuffer, encryptionKey)
}
} catch (error) {
if (error instanceof Error) {
toast.error(error.message)
}
console.error(error)
} finally {
setIsLoading(false) setIsLoading(false)
return
}
setLoadingSpinnerDesc('Generating encryption key')
const encryptionKey = await generateEncryptionKey()
if (await isOnline()) {
setLoadingSpinnerDesc('generating files.zip')
const arrayBuffer = await generateFilesZip()
if (!arrayBuffer) {
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Encrypting files.zip')
const encryptedArrayBuffer = await encryptZipFile(
arrayBuffer,
encryptionKey
)
setLoadingSpinnerDesc('Uploading files.zip to file storage')
const fileUrl = await uploadFile(encryptedArrayBuffer)
if (!fileUrl) {
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Generating create signature')
const createSignature = await generateCreateSignature(fileHashes, fileUrl)
if (!createSignature) {
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Generating keys for decryption')
// generate key pairs for decryption
const pubkeys = users.map((user) => user.pubkey)
// also add creator in the list
if (pubkeys.includes(usersPubkey!)) {
pubkeys.push(usersPubkey!)
}
const keys = await generateKeys(pubkeys, encryptionKey)
if (!keys) {
setIsLoading(false)
return
}
const meta: Meta = {
createSignature,
keys,
modifiedAt: unixNow(),
docSignatures: {}
}
setLoadingSpinnerDesc('Updating user app data')
const event = await updateUsersAppData(meta)
if (!event) {
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Sending notifications to counterparties')
const promises = sendNotifications(meta)
await Promise.all(promises)
.then(() => {
toast.success('Notifications sent successfully')
})
.catch(() => {
toast.error('Failed to publish notifications')
})
navigate(appPrivateRoutes.sign, { state: { meta: meta } })
} else {
const zip = new JSZip()
selectedFiles.forEach((file) => {
zip.file(`files/${file.name}`, file)
})
setLoadingSpinnerDesc('Generating create signature')
const createSignature = await generateCreateSignature(fileHashes, '')
if (!createSignature) {
setIsLoading(false)
return
}
const meta: Meta = {
createSignature,
modifiedAt: unixNow(),
docSignatures: {}
}
// add meta to zip
try {
const stringifiedMeta = JSON.stringify(meta, null, 2)
zip.file('meta.json', stringifiedMeta)
} catch (err) {
console.error(err)
toast.error('An error occurred in converting meta json to string')
return null
}
const arrayBuffer = await generateZipFile(zip)
if (!arrayBuffer) {
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Encrypting zip file')
const encryptedArrayBuffer = await encryptZipFile(
arrayBuffer,
encryptionKey
)
await handleOfflineFlow(encryptedArrayBuffer, encryptionKey)
} }
} }
@ -893,7 +893,7 @@ export const CreatePage = () => {
<ol className={`${styles.paperGroup} ${styles.orderedFilesList}`}> <ol className={`${styles.paperGroup} ${styles.orderedFilesList}`}>
{selectedFiles.length > 0 && {selectedFiles.length > 0 &&
selectedFiles.map((file, index) => ( selectedFiles.map((file, index) => (
<div <li
key={index} key={index}
className={`${fileListStyles.fileItem} ${isActive(file) && fileListStyles.active}`} className={`${fileListStyles.fileItem} ${isActive(file) && fileListStyles.active}`}
onClick={() => { onClick={() => {
@ -901,32 +901,32 @@ export const CreatePage = () => {
setCurrentFile(file) setCurrentFile(file)
}} }}
> >
<> <span className={styles.fileName}>{file.name}</span>
<span className={styles.fileName}>{file.name}</span> <Button
<Button aria-label={`delete ${file.name}`}
variant="text" variant="text"
onClick={(event) => handleRemoveFile(event, file)} onClick={(event) => handleRemoveFile(event, file)}
sx={{ sx={{
minWidth: '44px' minWidth: '44px'
}} }}
> >
<FontAwesomeIcon icon={faTrash} /> <FontAwesomeIcon icon={faTrash} />
</Button> </Button>
</> </li>
</div>
))} ))}
</ol> </ol>
<Button variant="contained" onClick={handleUploadButtonClick}> <Button variant="contained" onClick={handleUploadButtonClick}>
<FontAwesomeIcon icon={faUpload} /> <FontAwesomeIcon icon={faUpload} />
<span className={styles.uploadFileText}>Upload new files</span> <span className={styles.uploadFileText}>Upload new files</span>
<input
ref={fileInputRef}
hidden={true}
multiple={true}
type="file"
onChange={handleSelectFiles}
/>
</Button> </Button>
<input
ref={fileInputRef}
hidden={true}
multiple={true}
type="file"
aria-label="file-upload"
onChange={handleSelectFiles}
/>
</div> </div>
} }
right={ right={

View File

@ -542,10 +542,13 @@ export const SignPage = () => {
setLoadingSpinnerDesc('Signing nostr event') setLoadingSpinnerDesc('Signing nostr event')
const prevSig = getPrevSignersSig(hexToNpub(usersPubkey!)) const prevSig = getPrevSignersSig(hexToNpub(usersPubkey!))
if (!prevSig) return if (!prevSig) {
Review

Same here, re refactoring.

Same here, re refactoring.
setIsLoading(false)
toast.error('Previous signature is invalid')
return
}
const marks = getSignerMarksForMeta() const marks = getSignerMarksForMeta() || []
if (!marks) return
const signedEvent = await signEventForMeta({ prevSig, marks }) const signedEvent = await signEventForMeta({ prevSig, marks })
if (!signedEvent) return if (!signedEvent) return

View File

@ -0,0 +1,23 @@
import { Jsonable } from '.'
// Reuse common error messages for meta parsing
export enum MetaParseErrorType {
'PARSE_ERROR_EVENT' = 'error occurred in parsing the create signature event',
'PARSE_ERROR_SIGNATURE_EVENT_CONTENT' = "err in parsing the createSignature event's content"
}
export class MetaParseError extends Error {
public readonly context?: Jsonable
constructor(
message: string,
options: { cause?: Error; context?: Jsonable } = {}
) {
const { cause, context } = options
super(message, { cause })
this.name = this.constructor.name
this.context = context
}
}

29
src/types/errors/index.ts Normal file
View File

@ -0,0 +1,29 @@
export type Jsonable =
| string
| number
| boolean
| null
| undefined
| readonly Jsonable[]
| { readonly [key: string]: Jsonable }
| { toJSON(): Jsonable }
/**
* Handle errors
* Wraps the errors without message property and stringify to a message so we can use it later
* @param error
* @returns
*/
export function handleError(error: unknown): Error {
if (error instanceof Error) return error
// No message error, wrap it and stringify
let stringified = 'Unable to stringify the thrown value'
try {
stringified = JSON.stringify(error)
} catch (error) {
console.error(stringified, error)
}
return new Error(`Wrapped Error: ${stringified}`)
}

View File

@ -47,7 +47,7 @@ const filterMarksByPubkey = (marks: Mark[], pubkey: string): Mark[] => {
/** /**
* Takes Signed Doc Signatures part of Meta and extracts * Takes Signed Doc Signatures part of Meta and extracts
* all Marks into one flar array, regardless of the user. * all Marks into one flat array, regardless of the user.
* @param meta * @param meta
*/ */
const extractMarksFromSignedMeta = (meta: Meta): Mark[] => { const extractMarksFromSignedMeta = (meta: Meta): Mark[] => {

View File

@ -3,6 +3,11 @@ import { fromUnixTimestamp, parseJson } from '.'
import { Event, verifyEvent } from 'nostr-tools' import { Event, verifyEvent } from 'nostr-tools'
import { toast } from 'react-toastify' import { toast } from 'react-toastify'
import { extractFileExtensions } from './file' import { extractFileExtensions } from './file'
import { handleError } from '../types/errors'
import {
MetaParseError,
MetaParseErrorType
} from '../types/errors/MetaParseError'
export enum SignStatus { export enum SignStatus {
Signed = 'Signed', Signed = 'Signed',
@ -17,58 +22,6 @@ export enum SigitStatus {
Complete = 'Completed' Complete = 'Completed'
} }
type Jsonable =
| string
| number
| boolean
| null
| undefined
| readonly Jsonable[]
| { readonly [key: string]: Jsonable }
| { toJSON(): Jsonable }
export class SigitMetaParseError extends Error {
public readonly context?: Jsonable
constructor(
message: string,
options: { cause?: Error; context?: Jsonable } = {}
) {
const { cause, context } = options
super(message, { cause })
this.name = this.constructor.name
this.context = context
}
}
/**
* Handle meta errors
* Wraps the errors without message property and stringify to a message so we can use it later
* @param error
* @returns
*/
function handleError(error: unknown): Error {
if (error instanceof Error) return error
// No message error, wrap it and stringify
let stringified = 'Unable to stringify the thrown value'
try {
stringified = JSON.stringify(error)
} catch (error) {
console.error(stringified, error)
}
return new Error(`[SiGit Error]: ${stringified}`)
}
// Reuse common error messages for meta parsing
export enum SigitMetaParseErrorType {
'PARSE_ERROR_EVENT' = 'error occurred in parsing the create signature event',
'PARSE_ERROR_SIGNATURE_EVENT_CONTENT' = "err in parsing the createSignature event's content"
}
export interface SigitCardDisplayInfo { export interface SigitCardDisplayInfo {
createdAt?: number createdAt?: number
title?: string title?: string
@ -89,7 +42,7 @@ export const parseNostrEvent = async (raw: string): Promise<Event> => {
const event = await parseJson<Event>(raw) const event = await parseJson<Event>(raw)
return event return event
} catch (error) { } catch (error) {
throw new SigitMetaParseError(SigitMetaParseErrorType.PARSE_ERROR_EVENT, { throw new MetaParseError(MetaParseErrorType.PARSE_ERROR_EVENT, {
cause: handleError(error), cause: handleError(error),
context: raw context: raw
}) })
@ -109,8 +62,8 @@ export const parseCreateSignatureEventContent = async (
await parseJson<CreateSignatureEventContent>(raw) await parseJson<CreateSignatureEventContent>(raw)
return createSignatureEventContent return createSignatureEventContent
} catch (error) { } catch (error) {
throw new SigitMetaParseError( throw new MetaParseError(
SigitMetaParseErrorType.PARSE_ERROR_SIGNATURE_EVENT_CONTENT, MetaParseErrorType.PARSE_ERROR_SIGNATURE_EVENT_CONTENT,
{ {
cause: handleError(error), cause: handleError(error),
context: raw context: raw
@ -165,7 +118,7 @@ export const extractSigitCardDisplayInfo = async (meta: Meta) => {
return sigitInfo return sigitInfo
} catch (error) { } catch (error) {
if (error instanceof SigitMetaParseError) { if (error instanceof MetaParseError) {
toast.error(error.message) toast.error(error.message)
console.error(error.name, error.message, error.cause, error.context) console.error(error.name, error.message, error.cause, error.context)
} else { } else {