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,9 +17,11 @@ 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>
<div id="content-preview" className={styles.content}> <div id="content-preview" className={styles.content}>
{children} {children}
</div> </div>
</div>
<div className={styles.sidesWrap}> <div className={styles.sidesWrap}>
<div className={styles.sides}>{right}</div> <div className={styles.sides}>{right}</div>
</div> </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,15 +724,13 @@ export const CreatePage = () => {
} }
const handleCreate = async () => { const handleCreate = async () => {
try {
if (!validateInputs()) return 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
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Generating encryption key') setLoadingSpinnerDesc('Generating encryption key')
const encryptionKey = await generateEncryptionKey() const encryptionKey = await generateEncryptionKey()
@ -737,10 +738,7 @@ export const CreatePage = () => {
if (await isOnline()) { if (await isOnline()) {
setLoadingSpinnerDesc('generating files.zip') setLoadingSpinnerDesc('generating files.zip')
const arrayBuffer = await generateFilesZip() const arrayBuffer = await generateFilesZip()
if (!arrayBuffer) { if (!arrayBuffer) return
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Encrypting files.zip') setLoadingSpinnerDesc('Encrypting files.zip')
const encryptedArrayBuffer = await encryptZipFile( const encryptedArrayBuffer = await encryptZipFile(
@ -748,19 +746,19 @@ export const CreatePage = () => {
encryptionKey encryptionKey
) )
const markConfig = createMarks(fileHashes)
setLoadingSpinnerDesc('Uploading files.zip to file storage') setLoadingSpinnerDesc('Uploading files.zip to file storage')
const fileUrl = await uploadFile(encryptedArrayBuffer) const fileUrl = await uploadFile(encryptedArrayBuffer)
if (!fileUrl) { if (!fileUrl) return
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Generating create signature') setLoadingSpinnerDesc('Generating create signature')
const createSignature = await generateCreateSignature(fileHashes, fileUrl) const createSignature = await generateCreateSignature(
if (!createSignature) { markConfig,
setIsLoading(false) fileHashes,
return fileUrl
} )
if (!createSignature) return
setLoadingSpinnerDesc('Generating keys for decryption') setLoadingSpinnerDesc('Generating keys for decryption')
@ -772,11 +770,8 @@ export const CreatePage = () => {
} }
const keys = await generateKeys(pubkeys, encryptionKey) const keys = await generateKeys(pubkeys, encryptionKey)
if (!keys) return
if (!keys) {
setIsLoading(false)
return
}
const meta: Meta = { const meta: Meta = {
createSignature, createSignature,
keys, keys,
@ -786,10 +781,7 @@ export const CreatePage = () => {
setLoadingSpinnerDesc('Updating user app data') setLoadingSpinnerDesc('Updating user app data')
const event = await updateUsersAppData(meta) const event = await updateUsersAppData(meta)
if (!event) { if (!event) return
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Sending notifications to counterparties') setLoadingSpinnerDesc('Sending notifications to counterparties')
const promises = sendNotifications(meta) const promises = sendNotifications(meta)
@ -810,12 +802,15 @@ export const CreatePage = () => {
zip.file(`files/${file.name}`, file) zip.file(`files/${file.name}`, file)
}) })
const markConfig = createMarks(fileHashes)
setLoadingSpinnerDesc('Generating create signature') 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(fileHashes, '') const createSignature = await generateCreateSignature(
if (!createSignature) { markConfig,
setIsLoading(false) fileHashes,
return ''
} )
if (!createSignature) return
const meta: Meta = { const meta: Meta = {
createSignature, createSignature,
@ -834,10 +829,7 @@ export const CreatePage = () => {
} }
const arrayBuffer = await generateZipFile(zip) const arrayBuffer = await generateZipFile(zip)
if (!arrayBuffer) { if (!arrayBuffer) return
setIsLoading(false)
return
}
setLoadingSpinnerDesc('Encrypting zip file') setLoadingSpinnerDesc('Encrypting zip file')
const encryptedArrayBuffer = await encryptZipFile( const encryptedArrayBuffer = await encryptZipFile(
@ -847,6 +839,14 @@ export const CreatePage = () => {
await handleOfflineFlow(encryptedArrayBuffer, encryptionKey) await handleOfflineFlow(encryptedArrayBuffer, encryptionKey)
} }
} catch (error) {
if (error instanceof Error) {
toast.error(error.message)
}
console.error(error)
} finally {
setIsLoading(false)
}
} }
const onDrawFieldsChange = (sigitFiles: SigitFile[]) => { const onDrawFieldsChange = (sigitFiles: SigitFile[]) => {
@ -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,9 +901,9 @@ 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={{
@ -912,21 +912,21 @@ export const CreatePage = () => {
> >
<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>
</Button>
<input <input
ref={fileInputRef} ref={fileInputRef}
hidden={true} hidden={true}
multiple={true} multiple={true}
type="file" type="file"
aria-label="file-upload"
onChange={handleSelectFiles} onChange={handleSelectFiles}
/> />
</Button>
</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 {