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

View File

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

View File

@ -514,6 +514,9 @@ export const CreatePage = () => {
return (
file.pages?.flatMap((page, index) => {
return page.drawnFields.map((drawnField) => {
if (!drawnField.counterpart) {
throw new Error('Missing counterpart')
}
return {
type: drawnField.type,
location: {
@ -670,6 +673,7 @@ export const CreatePage = () => {
}
const generateCreateSignature = async (
markConfig: Mark[],
fileHashes: {
[key: string]: string
},
@ -677,7 +681,6 @@ export const CreatePage = () => {
) => {
const signers = users.filter((user) => user.role === UserRole.signer)
const viewers = users.filter((user) => user.role === UserRole.viewer)
const markConfig = createMarks(fileHashes)
const content: CreateSignatureEventContent = {
signers: signers.map((signer) => hexToNpub(signer.pubkey)),
@ -721,15 +724,13 @@ export const CreatePage = () => {
}
const handleCreate = async () => {
try {
if (!validateInputs()) return
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')
const fileHashes = await generateFileHashes()
if (!fileHashes) {
setIsLoading(false)
return
}
if (!fileHashes) return
setLoadingSpinnerDesc('Generating encryption key')
const encryptionKey = await generateEncryptionKey()
@ -737,10 +738,7 @@ export const CreatePage = () => {
if (await isOnline()) {
setLoadingSpinnerDesc('generating files.zip')
const arrayBuffer = await generateFilesZip()
if (!arrayBuffer) {
setIsLoading(false)
return
}
if (!arrayBuffer) return
setLoadingSpinnerDesc('Encrypting files.zip')
const encryptedArrayBuffer = await encryptZipFile(
@ -748,19 +746,19 @@ export const CreatePage = () => {
encryptionKey
)
const markConfig = createMarks(fileHashes)
setLoadingSpinnerDesc('Uploading files.zip to file storage')
const fileUrl = await uploadFile(encryptedArrayBuffer)
if (!fileUrl) {
setIsLoading(false)
return
}
if (!fileUrl) return
setLoadingSpinnerDesc('Generating create signature')
const createSignature = await generateCreateSignature(fileHashes, fileUrl)
if (!createSignature) {
setIsLoading(false)
return
}
const createSignature = await generateCreateSignature(
markConfig,
fileHashes,
fileUrl
)
if (!createSignature) return
setLoadingSpinnerDesc('Generating keys for decryption')
@ -772,11 +770,8 @@ export const CreatePage = () => {
}
const keys = await generateKeys(pubkeys, encryptionKey)
if (!keys) return
if (!keys) {
setIsLoading(false)
return
}
const meta: Meta = {
createSignature,
keys,
@ -786,10 +781,7 @@ export const CreatePage = () => {
setLoadingSpinnerDesc('Updating user app data')
const event = await updateUsersAppData(meta)
if (!event) {
setIsLoading(false)
return
}
if (!event) return
setLoadingSpinnerDesc('Sending notifications to counterparties')
const promises = sendNotifications(meta)
@ -810,12 +802,15 @@ export const CreatePage = () => {
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(fileHashes, '')
if (!createSignature) {
setIsLoading(false)
return
}
const createSignature = await generateCreateSignature(
markConfig,
fileHashes,
''
)
if (!createSignature) return
const meta: Meta = {
createSignature,
@ -834,10 +829,7 @@ export const CreatePage = () => {
}
const arrayBuffer = await generateZipFile(zip)
if (!arrayBuffer) {
setIsLoading(false)
return
}
if (!arrayBuffer) return
setLoadingSpinnerDesc('Encrypting zip file')
const encryptedArrayBuffer = await encryptZipFile(
@ -847,6 +839,14 @@ export const CreatePage = () => {
await handleOfflineFlow(encryptedArrayBuffer, encryptionKey)
}
} catch (error) {
if (error instanceof Error) {
toast.error(error.message)
}
console.error(error)
} finally {
setIsLoading(false)
}
}
const onDrawFieldsChange = (sigitFiles: SigitFile[]) => {
@ -893,7 +893,7 @@ export const CreatePage = () => {
<ol className={`${styles.paperGroup} ${styles.orderedFilesList}`}>
{selectedFiles.length > 0 &&
selectedFiles.map((file, index) => (
<div
<li
key={index}
className={`${fileListStyles.fileItem} ${isActive(file) && fileListStyles.active}`}
onClick={() => {
@ -901,9 +901,9 @@ export const CreatePage = () => {
setCurrentFile(file)
}}
>
<>
<span className={styles.fileName}>{file.name}</span>
<Button
aria-label={`delete ${file.name}`}
variant="text"
onClick={(event) => handleRemoveFile(event, file)}
sx={{
@ -912,21 +912,21 @@ export const CreatePage = () => {
>
<FontAwesomeIcon icon={faTrash} />
</Button>
</>
</div>
</li>
))}
</ol>
<Button variant="contained" onClick={handleUploadButtonClick}>
<FontAwesomeIcon icon={faUpload} />
<span className={styles.uploadFileText}>Upload new files</span>
</Button>
<input
ref={fileInputRef}
hidden={true}
multiple={true}
type="file"
aria-label="file-upload"
onChange={handleSelectFiles}
/>
</Button>
</div>
}
right={

View File

@ -542,10 +542,13 @@ export const SignPage = () => {
setLoadingSpinnerDesc('Signing nostr event')
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()
if (!marks) return
const marks = getSignerMarksForMeta() || []
const signedEvent = await signEventForMeta({ prevSig, marks })
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
* all Marks into one flar array, regardless of the user.
* all Marks into one flat array, regardless of the user.
* @param meta
*/
const extractMarksFromSignedMeta = (meta: Meta): Mark[] => {

View File

@ -3,6 +3,11 @@ import { fromUnixTimestamp, parseJson } from '.'
import { Event, verifyEvent } from 'nostr-tools'
import { toast } from 'react-toastify'
import { extractFileExtensions } from './file'
import { handleError } from '../types/errors'
import {
MetaParseError,
MetaParseErrorType
} from '../types/errors/MetaParseError'
export enum SignStatus {
Signed = 'Signed',
@ -17,58 +22,6 @@ export enum SigitStatus {
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 {
createdAt?: number
title?: string
@ -89,7 +42,7 @@ export const parseNostrEvent = async (raw: string): Promise<Event> => {
const event = await parseJson<Event>(raw)
return event
} catch (error) {
throw new SigitMetaParseError(SigitMetaParseErrorType.PARSE_ERROR_EVENT, {
throw new MetaParseError(MetaParseErrorType.PARSE_ERROR_EVENT, {
cause: handleError(error),
context: raw
})
@ -109,8 +62,8 @@ export const parseCreateSignatureEventContent = async (
await parseJson<CreateSignatureEventContent>(raw)
return createSignatureEventContent
} catch (error) {
throw new SigitMetaParseError(
SigitMetaParseErrorType.PARSE_ERROR_SIGNATURE_EVENT_CONTENT,
throw new MetaParseError(
MetaParseErrorType.PARSE_ERROR_SIGNATURE_EVENT_CONTENT,
{
cause: handleError(error),
context: raw
@ -165,7 +118,7 @@ export const extractSigitCardDisplayInfo = async (meta: Meta) => {
return sigitInfo
} catch (error) {
if (error instanceof SigitMetaParseError) {
if (error instanceof MetaParseError) {
toast.error(error.message)
console.error(error.name, error.message, error.cause, error.context)
} else {