mirror of
https://github.com/Permissionless-Software-Foundation/psf-memo-client.git
synced 2026-09-21 16:52:02 -07:00
Merge New Post Page broadcast-error surfacing fix
By specifier.
This commit is contained in:
@@ -31,7 +31,9 @@ function makeWallet (address) {
|
||||
return this.utxos
|
||||
},
|
||||
sendOpReturn: async function (walletInfo, bchUtxos, msg, prefix) {
|
||||
// Record the broadcast attempt, then fail if configured to do so.
|
||||
this.broadcasts.push({ walletInfo, bchUtxos, msg, prefix })
|
||||
if (this.failWith) throw new Error(this.failWith)
|
||||
return 'aa'.repeat(32)
|
||||
}
|
||||
}
|
||||
@@ -95,6 +97,17 @@ const handlers = [
|
||||
world.currentPath = NewPostPage.RECENT_FEED_PATH
|
||||
}
|
||||
},
|
||||
{
|
||||
name: 'wallet fails to broadcast with error',
|
||||
pattern: /^the wallet fails to broadcast with the error "<([A-Za-z0-9_]+)>"$/,
|
||||
run (m, example, world) {
|
||||
const param = m[1]
|
||||
if (!(param in example)) {
|
||||
throw new Error(`Missing example value for "${param}"`)
|
||||
}
|
||||
world.wallet.failWith = example[param]
|
||||
}
|
||||
},
|
||||
{
|
||||
name: 'navigate to path',
|
||||
pattern: /^I navigate to the path (.+)$/,
|
||||
@@ -109,6 +122,16 @@ const handlers = [
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
name: 'remain on path',
|
||||
pattern: /^I remain on the path (.+)$/,
|
||||
run (m, example, world) {
|
||||
const target = m[1].trim()
|
||||
if (world.currentPath !== target) {
|
||||
throw new Error(`Expected to remain on path ${target}, but current path is ${world.currentPath}.`)
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
name: 'open navigation menu',
|
||||
pattern: /^I open the navigation menu$/,
|
||||
@@ -145,8 +168,8 @@ const handlers = [
|
||||
}
|
||||
},
|
||||
{
|
||||
name: 'broadcasts OP_RETURN with Memo post prefix',
|
||||
pattern: /^(?:the wallet|the app) broadcasts an OP_RETURN transaction with the Memo post prefix$/,
|
||||
name: 'broadcasts/attempts OP_RETURN with Memo post prefix',
|
||||
pattern: /^(?:the wallet|the app) (?:broadcasts|attempts to broadcast) an OP_RETURN transaction with the Memo post prefix$/,
|
||||
run (m, example, world) {
|
||||
const broadcasts = world.wallet.broadcasts
|
||||
if (!broadcasts.length) {
|
||||
@@ -176,6 +199,18 @@ const handlers = [
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
name: 'page shows error containing text',
|
||||
pattern: /^the new post page shows an error containing "<([A-Za-z0-9_]+)>"$/,
|
||||
run (m, example, world) {
|
||||
const param = m[1]
|
||||
const expected = example[param]
|
||||
const actual = world.newPage.broadcastError || ''
|
||||
if (!actual.includes(expected)) {
|
||||
throw new Error(`Expected an error containing "${expected}", got "${actual}".`)
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
name: 'page shows validation/length error',
|
||||
pattern: /^the (?:app|new post page) shows a (validation|length) error$/,
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
# Architectural Review Summary — new-post-errors
|
||||
|
||||
## Task and commits reviewed
|
||||
- Task: `new-post-errors`
|
||||
- Reviewed the merged branch ending at `66a684923c` (refactorer), which carried:
|
||||
- `a125751`/`4e0f40c` — specifier broadcast-error surfacing spec (`specs/memo-new.feature`
|
||||
scenario 6)
|
||||
- `164bdb3` — coder implementation (surface broadcast errors on the new post page)
|
||||
- `66a6849` — refactorer failure-handling refactor + property coverage
|
||||
- Merged into `swarmforge-architect` (fast-forward) and processed as a batch.
|
||||
|
||||
## Architectural findings and fixes applied
|
||||
Reviewed UI/Core separation, dependency rule, information hiding/encapsulation, and
|
||||
local code quality.
|
||||
|
||||
1. **Failure classification (good).** `src/services/new-post.js` `_handleSubmitFailure`
|
||||
cleanly separates local validation failures (`submitError` = `memo_validation`/
|
||||
`memo_length`) from broadcast/handler failures (surfaced via `broadcastError` and a
|
||||
`broadcast` submit state). The controller stays on the page on failure; the UI
|
||||
component surfaces the real message. Injection of `memoPost`/`navigate` keeps it
|
||||
free of UI/IO concerns; dependency direction is inward.
|
||||
2. **Unified handlers extended (good).** `acceptance/lib/handlers.js` adds a
|
||||
`wallet fails to broadcast` step, a `remain on path` assertion, and
|
||||
`attempts to broadcast`/`shows an error containing` patterns without duplicating
|
||||
step logic; the two features share one `world.newPage`.
|
||||
3. **Property coverage (good).** A seeded property asserts a broadcast failure never
|
||||
navigates and always surfaces a `broadcast` submitError with the real error text.
|
||||
4. **Fix applied — error-message fallback coverage.** The language mutation tool
|
||||
surfaced one survivor on `new-post.js:87` (`err.message || String(err)` → `&&`),
|
||||
which only matters when `err.message` is falsy. No test exercised that path. Added
|
||||
a unit test where a broadcast throws an empty-message `Error` and asserts the string
|
||||
form is surfaced, killing the mutant.
|
||||
|
||||
## Verification results
|
||||
- **Unit (`node --test`):** 21/21 pass (added the empty-message fallback test).
|
||||
- **Property (`npm run test:property`):** 7/7 pass.
|
||||
- **Acceptance (normal):** both `memo-new` and `post-memo` generated suites pass,
|
||||
including scenario 6 (broadcast error surfaced, user stays on page).
|
||||
- **Mutation (`mutate4javascript`, `--max-workers 8`, `--mutate-all`):**
|
||||
`new-post.js` — **killed 12 / survived 0 / uncovered 0** (was 1 survivor before the
|
||||
fix).
|
||||
- **DRY (`dry4javascript src`):** no duplicate candidates.
|
||||
- **Gherkin acceptance mutation (soft):** `memo-new.feature` — 14 executed,
|
||||
**4 killed, 10 survived**, 0 errors (1 previously-killed empty-memo scenario reused).
|
||||
- Killed: character-counter `count` values and the empty-memo boundary — values are
|
||||
behaviorally connected.
|
||||
- Survived (documented equivalents): message-text dithers and broadcast-error-text
|
||||
dithers; these are opaque data or substring-consistent with the surfaced error, so
|
||||
they do not change the exercised branch.
|
||||
- Property tests run separately via `npm run test:property`.
|
||||
|
||||
## Suite status
|
||||
- Unit + property + acceptance all pass; source-level mutation fully kills both testable
|
||||
core modules. Gherkin acceptance mutation survivors are documented equivalents.
|
||||
|
||||
## Handoffs sent
|
||||
- `git_handoff` → coder, refactorer (priority `00`, task `new-post-errors`), to review
|
||||
the architect commit (fallback coverage + tool manifests).
|
||||
|
||||
By architect.
|
||||
@@ -1,5 +1,5 @@
|
||||
# acceptance-mutation-manifest-begin
|
||||
# {"version":1,"tested_at":"2026-08-26T00:08:15.433121898Z","feature_name":"New Post Page","feature_path":"/home/trout/work/psf-memo-client/.worktrees/architect/specs/memo-new.feature","background_hash":"e1d5f81f1ed083ac6934c429ca3cb4a0f8d4dac44c2eaa45c0960920bde2c017","implementation_hash":"unknown","scenarios":[{"index":1,"name":"New Post Page - 2 an empty memo is rejected on the new post page","scenario_hash":"ac70dcf123f435da2b8a6c4953b0b22b8e7a5a8a74291547b954cb562cf9f339","mutation_count":1,"result":{"Total":1,"Killed":1,"Survived":0,"Errors":0},"tested_at":"2026-08-26T00:08:15.433121898Z"}]}
|
||||
# {"version":1,"tested_at":"2026-08-26T00:40:19.414904151Z","feature_name":"New Post Page","feature_path":"/home/trout/work/psf-memo-client/.worktrees/architect/specs/memo-new.feature","background_hash":"e1d5f81f1ed083ac6934c429ca3cb4a0f8d4dac44c2eaa45c0960920bde2c017","implementation_hash":"unknown","scenarios":[{"index":1,"name":"New Post Page - 2 an empty memo is rejected on the new post page","scenario_hash":"ac70dcf123f435da2b8a6c4953b0b22b8e7a5a8a74291547b954cb562cf9f339","mutation_count":1,"result":{"Total":1,"Killed":1,"Survived":0,"Errors":0},"tested_at":"2026-08-26T00:08:15.433121898Z"}]}
|
||||
# acceptance-mutation-manifest-end
|
||||
|
||||
# Scenarios: New Post Page - 1, New Post Page - 2, New Post Page - 3, New Post Page - 4, New Post Page - 5, New Post Page - 6
|
||||
|
||||
@@ -36,11 +36,15 @@ function NewPost (props) {
|
||||
|
||||
const result = await page.submit()
|
||||
if (!result.ok) {
|
||||
setErr(
|
||||
result.error === 'memo_length'
|
||||
? `Memo is too long. Maximum is ${maxChars} characters.`
|
||||
: 'Memo must not be empty.'
|
||||
)
|
||||
if (result.error === 'memo_length') {
|
||||
setErr(`Memo is too long. Maximum is ${maxChars} characters.`)
|
||||
} else if (result.error === 'memo_validation') {
|
||||
setErr('Memo must not be empty.')
|
||||
} else if (result.message) {
|
||||
setErr(`Failed to broadcast: ${result.message}`)
|
||||
} else {
|
||||
setErr('Failed to post memo.')
|
||||
}
|
||||
}
|
||||
// On success page.submit() navigated to the recent feed.
|
||||
} catch (submitErr) {
|
||||
|
||||
@@ -26,6 +26,7 @@ class NewPostPage {
|
||||
|
||||
this.input = ''
|
||||
this.submitError = null
|
||||
this.broadcastError = null
|
||||
this.posting = false
|
||||
|
||||
// The navigation menu links to the new post page.
|
||||
@@ -55,10 +56,12 @@ class NewPostPage {
|
||||
}
|
||||
|
||||
// Validate and post the current draft. On success, navigate to the recent
|
||||
// feed. On failure, record the typed error. Resolves with a result object.
|
||||
// feed. On failure, record the typed error and stay on the page. Resolves
|
||||
// with a result object.
|
||||
async submit () {
|
||||
this.posting = true
|
||||
this.submitError = null
|
||||
this.broadcastError = null
|
||||
|
||||
try {
|
||||
if (!this.memoPost) {
|
||||
@@ -70,11 +73,23 @@ class NewPostPage {
|
||||
this.posting = false
|
||||
return { ok: true, txid }
|
||||
} catch (err) {
|
||||
this.submitError = err.code || 'memo_validation'
|
||||
this.posting = false
|
||||
return { ok: false, error: this.submitError }
|
||||
return this._handleSubmitFailure(err)
|
||||
}
|
||||
}
|
||||
|
||||
// Classify a submit failure, record the typed state, and return the failure
|
||||
// result. Local validation failures set submitError; broadcast or handler
|
||||
// failures surface the real error message via broadcastError.
|
||||
_handleSubmitFailure (err) {
|
||||
if (err.code === 'memo_validation' || err.code === 'memo_length') {
|
||||
this.submitError = err.code
|
||||
} else {
|
||||
this.broadcastError = err.message || String(err)
|
||||
this.submitError = 'broadcast'
|
||||
}
|
||||
this.posting = false
|
||||
return { ok: false, error: this.submitError, message: this.broadcastError }
|
||||
}
|
||||
}
|
||||
|
||||
NewPostPage.NEW_POST_PATH = NEW_POST_PATH
|
||||
@@ -83,5 +98,5 @@ NewPostPage.RECENT_FEED_PATH = RECENT_FEED_PATH
|
||||
module.exports = NewPostPage
|
||||
|
||||
// mutate4javascript-manifest-begin
|
||||
// {"version":1,"tested_at":"2026-08-26T00:07:51.843Z","module_hash":"469dfd90342f6bcacf5b89ed819620663e221e20832f6936c35c46f9681ebfdc","functions":[{"id":"func/NewPostPage.constructor","name":"NewPostPage.constructor","line":22,"end_line":33,"hash":"7c957fbaa2b8d4adb62c1bbf240749243696a68e71e7896aefdbb68437432cd8"},{"id":"func/NewPostPage.addMenuLink","name":"NewPostPage.addMenuLink","line":36,"end_line":39,"hash":"bac97164d2d70bfdb946c4d54e67983c43cad093bac558f1d169e1739ca97137"},{"id":"func/NewPostPage.hasMenuLink","name":"NewPostPage.hasMenuLink","line":42,"end_line":44,"hash":"7e1abf5d0833aaf3b3da3a024de9eb2b80da900b2930e832c7e92d77e0e50344"},{"id":"func/NewPostPage.setInput","name":"NewPostPage.setInput","line":47,"end_line":50,"hash":"595484662b7ca07ef5eef5cebbff06309242552d9b4d15687df02f260ba88244"},{"id":"func/NewPostPage.remainingCount","name":"NewPostPage.remainingCount","line":53,"end_line":55,"hash":"521ce4ed841f62099529b327f2245a9e94c09af2f67ed5d91839e52607bcea37"},{"id":"func/NewPostPage.submit","name":"NewPostPage.submit","line":59,"end_line":77,"hash":"f4ce743f5a4bb139615086165b25173641a9388af16b7527f2db243a1c6b596c"}]}
|
||||
// {"version":1,"tested_at":"2026-08-26T00:39:54.163Z","module_hash":"8d50d002e9c6094a1bd2d6e764023c942b1eb0f085b019255dc80f0a72ab1ec6","functions":[{"id":"func/NewPostPage.constructor","name":"NewPostPage.constructor","line":22,"end_line":34,"hash":"d61d01986c51dc4ed4185594fa3e35612924846db321a7107aaec191d17c419d"},{"id":"func/NewPostPage.addMenuLink","name":"NewPostPage.addMenuLink","line":37,"end_line":40,"hash":"bac97164d2d70bfdb946c4d54e67983c43cad093bac558f1d169e1739ca97137"},{"id":"func/NewPostPage.hasMenuLink","name":"NewPostPage.hasMenuLink","line":43,"end_line":45,"hash":"7e1abf5d0833aaf3b3da3a024de9eb2b80da900b2930e832c7e92d77e0e50344"},{"id":"func/NewPostPage.setInput","name":"NewPostPage.setInput","line":48,"end_line":51,"hash":"595484662b7ca07ef5eef5cebbff06309242552d9b4d15687df02f260ba88244"},{"id":"func/NewPostPage.remainingCount","name":"NewPostPage.remainingCount","line":54,"end_line":56,"hash":"521ce4ed841f62099529b327f2245a9e94c09af2f67ed5d91839e52607bcea37"},{"id":"func/NewPostPage.submit","name":"NewPostPage.submit","line":61,"end_line":78,"hash":"c280ee1244bcb80c3a9ffe4f52befc8a05629ec879ef9d86666ea11f493b3c5b"},{"id":"func/NewPostPage._handleSubmitFailure","name":"NewPostPage._handleSubmitFailure","line":83,"end_line":92,"hash":"b13d8cd6b48b1f42d72e0031cabcaa00bb78b813f42250517d711f0d6fb23126"}]}
|
||||
// mutate4javascript-manifest-end
|
||||
|
||||
@@ -34,6 +34,25 @@ function buildPage () {
|
||||
})
|
||||
}
|
||||
|
||||
// A fake wallet recording broadcast attempts; fails when failWith is set.
|
||||
function fakeWallet () {
|
||||
const wallet = {
|
||||
walletInfo: { cashAddress: 'bitcoincash:qqlrzp23w08434twmvr4fxw672whkjy0py26r63g3d' },
|
||||
utxos: [{ txid: 'utxo-fee' }],
|
||||
getUtxos: async function () { return this.utxos },
|
||||
sendOpReturn: async function (walletInfo, bchUtxos, msg, prefix) {
|
||||
if (this.failWith) throw new Error(this.failWith)
|
||||
return 'prop-txid'
|
||||
}
|
||||
}
|
||||
return wallet
|
||||
}
|
||||
|
||||
function fakeFeed () {
|
||||
const posts = []
|
||||
return { posts, addPost: (p) => posts.push(p) }
|
||||
}
|
||||
|
||||
test('memo validation: any non-blank string at or below the limit is valid', async () => {
|
||||
await forAll(
|
||||
(i) => {
|
||||
@@ -110,3 +129,27 @@ test('menu link registration is idempotent', async () => {
|
||||
{ label: 'menu link idempotence' }
|
||||
)
|
||||
})
|
||||
|
||||
test('a broadcast failure surfaces the error and never navigates', async () => {
|
||||
await forAll(
|
||||
(i) => ({ message: stringOf(1 + Math.floor(rng() * 40)), failWith: `boom-${i % 97}` }),
|
||||
({ message, failWith }) => {
|
||||
const wallet = fakeWallet()
|
||||
wallet.failWith = failWith
|
||||
const navigations = []
|
||||
const page = new NewPostPage({
|
||||
memoPost: new MemoPost({ wallet, feed: fakeFeed() }),
|
||||
navigate: (p) => navigations.push(p)
|
||||
})
|
||||
page.setInput(message)
|
||||
|
||||
return page.submit().then((result) => {
|
||||
if (result.ok) return false
|
||||
if (page.submitError !== 'broadcast') return false
|
||||
if (!page.broadcastError || !page.broadcastError.includes('boom')) return false
|
||||
return navigations.length === 0
|
||||
})
|
||||
},
|
||||
{ label: 'broadcast failure does not navigate' }
|
||||
)
|
||||
})
|
||||
|
||||
@@ -29,6 +29,7 @@ function fakeWallet (cashAddress = 'bitcoincash:qqlrzp23w08434twmvr4fxw672whkjy0
|
||||
getUtxos: async function () { return this.utxos },
|
||||
sendOpReturn: async function (walletInfo, bchUtxos, msg, prefix) {
|
||||
this.broadcasts.push({ walletInfo, bchUtxos, msg, prefix })
|
||||
if (this.failWith) throw new Error(this.failWith)
|
||||
return 'newpost-txid'
|
||||
}
|
||||
}
|
||||
@@ -154,7 +155,7 @@ test('posting is true while a submit is in flight and false once it settles', as
|
||||
assert.equal(page.posting, true)
|
||||
|
||||
// Yield until the async chain reaches the deferred sendOpReturn call.
|
||||
await new Promise((r) => setImmediate(r))
|
||||
await new Promise((resolve) => setImmediate(resolve))
|
||||
assert.equal(typeof resolveSend, 'function')
|
||||
resolveSend('in-flight-txid')
|
||||
await pending
|
||||
@@ -171,3 +172,57 @@ test('submitting without a memo post handler reports an error and does not navig
|
||||
assert.equal(result.ok, false)
|
||||
assert.deepEqual(navigations, [])
|
||||
})
|
||||
|
||||
test('a failed broadcast surfaces the real error and does not navigate', async () => {
|
||||
const wallet = fakeWallet()
|
||||
const feed = fakeFeed()
|
||||
wallet.failWith = 'BCH UTXO list is empty'
|
||||
const navigations = []
|
||||
const page = new NewPostPage({
|
||||
memoPost: new MemoPost({ wallet, feed }),
|
||||
navigate: (p) => navigations.push(p)
|
||||
})
|
||||
page.setInput('hello memo')
|
||||
|
||||
const result = await page.submit()
|
||||
|
||||
assert.equal(result.ok, false)
|
||||
assert.equal(page.submitError, 'broadcast')
|
||||
assert.match(page.broadcastError, /BCH UTXO list is empty/)
|
||||
// The broadcast was attempted (recorded) before it failed.
|
||||
assert.equal(wallet.broadcasts.length, 1)
|
||||
assert.equal(wallet.broadcasts[0].prefix, '6d02')
|
||||
// The user stays on the page.
|
||||
assert.deepEqual(navigations, [])
|
||||
})
|
||||
|
||||
test('a failed broadcast surfaces a different real error message', async () => {
|
||||
const wallet = fakeWallet()
|
||||
wallet.failWith = 'Insufficient balance'
|
||||
const page = new NewPostPage({
|
||||
memoPost: new MemoPost({ wallet }),
|
||||
navigate: () => {}
|
||||
})
|
||||
page.setInput('hello memo')
|
||||
|
||||
const result = await page.submit()
|
||||
|
||||
assert.equal(result.ok, false)
|
||||
assert.match(page.broadcastError, /Insufficient balance/)
|
||||
})
|
||||
|
||||
test('a broadcast failure with an empty message falls back to a string form', async () => {
|
||||
const wallet = fakeWallet()
|
||||
// Throw an Error with an empty message so the message fallback path is exercised.
|
||||
wallet.sendOpReturn = async () => { throw new Error('') }
|
||||
const page = new NewPostPage({ memoPost: new MemoPost({ wallet }), navigate: () => {} })
|
||||
page.setInput('hello memo')
|
||||
|
||||
const result = await page.submit()
|
||||
|
||||
assert.equal(result.ok, false)
|
||||
assert.equal(page.submitError, 'broadcast')
|
||||
// The real (string) error is surfaced even though the message was empty.
|
||||
assert.equal(typeof page.broadcastError, 'string')
|
||||
assert.ok(page.broadcastError.length > 0)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user