Skip to content

fix: tech-debt pass — leaked listeners, async error handling, query guards, dead code - #14665

Closed
dylanjeffers wants to merge 6 commits into
mainfrom
claude/charming-ptolemy-57tthc
Closed

dylanjeffers wants to merge 6 commits into
mainfrom
claude/charming-ptolemy-57tthc

Conversation

@dylanjeffers

@dylanjeffers dylanjeffers commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Code review and tech-debt pass over sdk, harmony, common, mobile and web. There is one commit per package.

sdk (patch changeset: .changeset/sdk-error-handling-fixes.md)

  • Storage uploads: tus doesn't await onSuccess, so a polling failure became an unhandled rejection and start() never settled.
  • Aborted uploads: the abort check was swallowed inside the try, so an aborted upload kept polling every 3s for up to an hour.
  • StorageNodeSelector:
    • The constructor's refresh was fire-and-forget, so a failed /health_check caused an unhandled rejection, which kills a Node process.
    • If a selection ran before the refresh, the node ordering stayed stale forever.
  • Request signature middleware: after logout or a signing failure, it kept attaching the previous user's cached signature.
  • SolanaRelayWalletAdapter: connecting stayed true forever if getFeePayer threw.
  • SolanaRelay: a non-JSON or null error body hid the real ResponseError.
  • SolanaClient: a fractional priority fee made BigInt() throw a RangeError, so the fee is now rounded.
  • ChatsApi:
    • A rejected public-key lookup was cached, which broke create and invite until reload.
    • Websocket handler rejections went unhandled.
  • Removed unused internal utils and updated terminology to validator node. generatePreview now throws No validator node available.

harmony

  • Modal: the touchmove listener leaked on unmount and stacked up when onTouchMove or modalRoot changed.
  • Slider: document listeners leaked when it unmounted mid-drag.
  • Removed the dead FilterButtonKeyHandler; findAncestor now uses closest().

common

  • Query guards: enabled: guard, ...options let a caller's { enabled: true } drop the hook's own guard in about 15 hooks.
    • Examples: useCurrentAccount ran with an undefined wallet, and useCoinBalance fired the AUDIO queries for non-AUDIO mints.
    • These now use enabled: guard && options.enabled !== false, matching useUser.
  • useMuteUser: rollback reset query keys that don't exist, so muted comments stayed hidden after a failed mute.
  • useMarkNotificationsAsViewed: restore the unread count when the request fails.
  • uuid(): the variant digit came from 'a' & 0x3, which is NaN & 3, so it was biased.
  • Duplicated upload logic: the backdrop image upload and validator-node URL code was copied in common and twice in web. It's now one uploadBackdropImage helper, with tests.
  • Dead code removed:
    • 6 unused files and about 50 unused exports.
    • 3 feature flags nobody reads: IOS_USDC_PURCHASE_ENABLED, REACT_QUERY_SYNC, COLLAPSED_EXPLORE_HEADER.
    • 29 unused remote-config keys.
    • Each removal was grepped across all packages first.
  • Replaced several @ts-ignores with proper types.

mobile

  • GoogleCast: the PlaybackActiveTrackChanged listener was never removed, so it piled up per reconnect or track change and kept calling stale cast clients.
  • AirPlay: cleanup called a nonexistent .stop(), so the listener was never removed.
  • useRevertOnCancel: the beforeRemove listener was never unsubscribed.
  • audius-sdk: every concurrent init waiter added a permanent listener; it now uses once().
  • ChatScreen: the AppState listener was re-added on every render.
  • useEnterContest: if the artwork prefetch failed, tapping did nothing. It now opens Upload without the artwork prefilled.
  • Shared the skip/restart constants and removed 10 unneeded @ts-ignores.

web

  • WithdrawalsTab: the withdrawal poller never stopped. reset (new identity every render) and transactions were effect deps, so the effect restarted on every render. That also restarted the 60s timeout, and if the transaction never appeared it called resetQueries every second forever.
  • StripeOnRampModal:
    • The onramp_session_updated listener was never removed, so it stacked up on each reopen with the same session and dispatched status changes more than once.
    • A failed session init was an unhandled rejection; it's now caught and logged.
  • foregroundPollingDaemon: the visibilitychange listener is now removed when the saga is cancelled.
  • Removed unused useScript, fixed the hooks README, and updated terminology.

Found but not fixed

  • harmony useHotKeys: builds a new throttle() on every keydown, so it never throttles. This is public API behavior, so it's left alone.
  • harmony useModalRoot: a modal unmounted while open leaves its root, container and backdrop divs in document.body.
  • common useUSDCTransactions: queryKey is rebuilt every render, so reset is unstable for every consumer. Memoizing the key would fix it at the source.
  • mobile SearchExploreScreen: an effect with no deps array scrolls to the top on every render while the query is one character or less.
  • mobile usePlaybackPositionPersistence: JSON.parse has no try/catch.
  • Feature flags defaulting to true (USDC_PURCHASES, LAUNCHPAD_VERIFICATION, WEEKLY_ROTATION): their off-branches are left in place.
  • Dead files not deleted in this PR:
    • About 57 in mobile, including the unused keyboard redux slice.
    • About 80 source files in web.
    • About 4MB of unused assets in web/src/assets, including all of assets/fonts. AnimatedIconButton's dynamic import() makes Vite emit a chunk for every JSON in the animations folder, so the unused animations also add build output.
    • These are worth a separate cleanup PR.
  • Pre-existing failure: common store/upload/selectors.test.ts was already failing (7 failures); its files aren't touched here.

Testing

  • sdk:
    • tsc: 0 errors. eslint: clean.
    • vitest: 216 passed, 2 skipped.
    • One StorageNodeSelector test needs NO_PROXY in the sandbox because axios goes through the proxy; that's unrelated to this PR.
  • harmony: tsc and eslint clean.
  • common:
    • tsc and eslint clean.
    • vitest: 30 of 31 files pass; the one failure is the pre-existing file above.
  • mobile:
    • tsc: 0 errors.
    • eslint: same result as before this PR.
    • jest: 34 of 34 pass.
  • web:
    • tsc: 0 errors.
    • eslint: clean on changed files.
    • No unit tests cover the touched files.
  • Not tested on a device or in a browser.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J37cvyRSg4MeLrVoQA1Uhh

…hats

- Storage: route tus onSuccess polling failures to reject; stop polling after abort
- StorageNodeSelector: catch constructor refresh rejection; reset ordering on refresh
- Request signature middleware: clear cached signature on signing errors
- SolanaRelayWalletAdapter: reset connecting flag when getFeePayer throws
- SolanaRelay: don't mask ResponseError with body parse errors
- SolanaClient: round priority fee so BigInt() doesn't throw
- ChatsApi: don't cache rejected public key lookups; catch websocket handler errors
- Remove unused internal utils; validator node terminology in comments/logs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J37cvyRSg4MeLrVoQA1Uhh
- Don't let caller options override hooks' enabled guards
- useMuteUser: reset the real comment list/count query keys on error
- useMarkNotificationsAsViewed: restore unread count on error
- uuid(): fix variant digit computed from a string with a bitwise op
- Consolidate backdrop image upload into uploadBackdropImage helper (common + web)
- Remove unused files, exports, feature flags and remote config keys
- Replace @ts-ignores with types; validator node terminology in comments

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J37cvyRSg4MeLrVoQA1Uhh
- GoogleCast/Airplay: remove track/device listeners on cleanup
- useRevertOnCancel: actually unsubscribe the beforeRemove listener
- audius-sdk: use once() for init waiters
- ChatScreen: stop re-adding the AppState listener every render
- useEnterContest: handle artwork prefetch failures
- Share skip/restart constants; drop unneeded @ts-ignores

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J37cvyRSg4MeLrVoQA1Uhh
@gitguardian

gitguardian Bot commented Oct 12, 2026

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
32408190 Triggered Generic High Entropy Secret 040835a packages/sdk/src/sdk/middleware/addRequestSignatureMiddleware.test.ts View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@changeset-bot

changeset-bot Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8449b61

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@audius/sdk Patch
@audius/sdk-legacy Patch
@audius/protocol-dashboard Patch
@audius/sp-actions Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Avoids adding another copy of the hardcoded test key that secret scanning flags.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J37cvyRSg4MeLrVoQA1Uhh

Copy link
Copy Markdown
Contributor Author

GitGuardian finding 32408190 (addRequestSignatureMiddleware.test.ts) is a throwaway test-only wallet key, not a real credential. The same key is already on main twice in that test file. b08ee74 changes the new test to use generatePrivateKey(), so this PR no longer adds another copy of the key. Nothing needs rotating.


Generated by Claude Code

- WithdrawalsTab: the poller restarted (and reset its 60s timeout) on every
  render because reset/transactions were effect deps, so it never stopped
- StripeOnRampModal: remove onramp_session_updated listener on cleanup;
  catch session init failures
- foregroundPollingDaemon: remove visibilitychange listener when cancelled
- Remove unused useScript; fix hooks README; validator node terminology

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J37cvyRSg4MeLrVoQA1Uhh
dylanjeffers added a commit that referenced this pull request Nov 12, 2026
…inology (#14774)

First of five PRs that re-ship #14665 on current main, ordered from
lowest to highest risk. Each one is branched off main and they don't
depend on each other.

## What's in it

Code removal and type/comment cleanup only. Nothing here should change
runtime behavior.

**sdk** (internal files only, nothing exported from the package entry
point)
- Delete unused `utils/deepPartial`, `utils/localStorage`,
`utils/object`, `utils/uuid`.
- Delete unused `getNStorageNodes`; `isNodeHealthy` moves to its own
file unchanged.
- `EntityManagerClient` uses the existing `wait()` helper.
- Comments and one error string say "validator node" instead of
discovery/content node. `generatePreview` now throws `No validator node
available`. The relay quote error names the right param
(`audioInputAmount`).

**harmony**
- Delete unused `FilterButtonKeyHandler` (not exported) and an empty
`useEffect` in `FollowButton`.
- `findAncestor` is just `el.closest()`. The fallback was for browsers
without `closest`.

**common**
- Delete unused files: `useUpdateCollection`, `useUnfollowUser`,
`usePremiumTracks`, `reactions/utils`, `models/Activity`, the empty
`formatTrackTimestamp`.
- Delete about 40 unused exports (adapters, saga helpers, selector
helpers, string/format/uid/url utils, jupiter
`addTransferToUserBankInstructions` and `isDirectRouteAvailable`, AUDIO
balance sagas, and others).
- Replace `@ts-ignore`s with types in the notification adapter,
`useInstanceVar`, `commonPasswordCheck` and `useUSDCTransactions`
(`Object.assign` onto the same result object, as before).
- Comment cleanup: commented-out saga list, stale TODOs, validator node
terminology.

**mobile**
- Skip/restart constants shared between `AudioPlayer` and
`NowPlayingDrawer` in `components/audio/constants.ts`.
- Remove 10 `@ts-ignore`s that are no longer needed, and some dead
comments.

**web**
- Delete unused `useScript`; fix the hooks README.
- Terminology in a sign-up comment and the dev tools Healthz
description.

Every removed export, file and flag was re-grepped across all packages
on current main before deleting.

## Why this tier

Deletions of code with no callers, type-only changes and comments. If
something is wrong here it fails at compile time, not at runtime.

## Dropped from #14665 because main already covers it
- Remote-config keys and the `IOS_USDC_PURCHASE_ENABLED`,
`REACT_QUERY_SYNC`, `COLLAPSED_EXPLORE_HEADER` flags: already removed on
main (#14671 and flag cleanups).
- `SLIPPAGE_TOLERANCE_EXCEEDED_ERROR` in `services/Jupiter.ts`: already
removed.
- sdk `SolanaAddressSchema` `@ts-ignore`: main rewrote it with a base58
check.
- Prettier-only reformatting in `useTrackPageLineup`, `WalletClient` and
`storeContext`.

## Verified
- `tsc`: sdk and harmony clean. common, mobile and web each show the
same errors as origin/main in this checkout, all in untouched files (a
vitest type in a common test, `SelectablePillField` in web,
`ThemeProvider` in mobile).
- `eslint` on changed files: clean.
- sdk vitest: 31 files, 249 passed, 2 skipped.
- common vitest: 40 of 41 files pass. `store/upload/selectors.test.ts`
(7 tests) fails the same way on origin/main.
- mobile jest: 18 of 18 suites pass.

## Not verified
Not run on a device or in a browser.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
dylanjeffers added a commit that referenced this pull request Nov 12, 2026
…jections (#14775)

Second of five PRs that re-ship #14665 on current main, ordered from
lowest to highest risk. Branched off main, independent of the others.

## What's in it

Listener leaks and unhandled rejections. None of these are meant to
change what the user sees.

**harmony**
- `Modal`: the `touchmove` listener on the modal root was only removed
on the next close render, so it leaked on unmount and stacked when
`onTouchMove` or `modalRoot` changed. It's now removed in the effect
cleanup.
- `Slider`: document mouse/touch listeners leaked when it unmounted
mid-drag. They're removed on unmount.

**mobile**
- `GoogleCast`: the `PlaybackActiveTrackChanged` listener was never
removed, so it piled up on each reconnect or track change and kept
calling stale cast clients. Moved from `useAsync` to `useEffect` with a
cleanup, and load errors are logged.
- `Airplay`: cleanup called `.stop()`, which doesn't exist on the
subscription, so the listener was never removed. It now calls
`.remove()`.
- `useRevertOnCancel`: `navigation.addListener` returns an unsubscribe
function, and passing that to `removeListener` did nothing. The effect
now returns the unsubscribe.
- `audius-sdk`: each concurrent init waiter added a permanent listener.
It uses `once()`.
- `ChatScreen`: the AppState listener effect had no deps array, so it
was removed and re-added on every render.

**web**
- `StripeOnRampModal`: the `onramp_session_updated` listener is removed
on cleanup (it stacked on each reopen with the same session), and a
failed session init is caught and logged.
- `foregroundPollingDaemon`: the `visibilitychange` listener is removed
when the saga is cancelled.

**common**
- `uuid()`: the variant digit came from `'a' & 0x3`, which is `NaN & 3`,
so hex letters always became `8`. It now parses the digit first. Output
format is unchanged. This was in tier 3 in the plan; I moved it here
because it only changes the random distribution of one character.

## Why this tier

Each fix is local to one component or helper and only touches cleanup
paths or error handling.

## Verified
- `tsc`: harmony clean. common, mobile and web show the same errors as
origin/main in this checkout, all in untouched files.
- `eslint` on changed files: clean.
- common vitest and mobile jest: same results as tier 1 (common's
pre-existing `store/upload/selectors.test.ts` failure only).

## Not verified
Not run on a device or in a browser. Chromecast, AirPlay and the Stripe
onramp in particular were not exercised.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
dylanjeffers added a commit that referenced this pull request Nov 12, 2026
…, backdrop upload helper (#14776)

Third of five PRs that re-ship #14665 on current main, ordered from
lowest to highest risk. Branched off main, independent of the others.

## What's in it

App-level fixes that change behavior in specific flows.

- **web `WithdrawalsTab`**: after a withdrawal, the poller that waits
for the new transaction listed `reset` (a new function every render) and
`transactions` as effect deps. Every render restarted it, including its
60s timeout, so if the transaction never appeared it called
`resetQueries` every second for as long as the page was open. It now
reads both through refs, restarts only when the awaited transaction
changes, and stops after 60s.
- **common `useMuteUser`**: the error rollback reset
`['trackCommentList', trackId, currentSort]`, but the real key is `[...,
{ sortMethod }]`, so muted comments stayed hidden after a failed mute.
It now uses `getTrackCommentListQueryKey` and
`getTrackCommentCountQueryKey`.
- **common `useMarkNotificationsAsViewed`**: restores the previous
unread count if the request fails, instead of leaving the badge at 0.
- **common `uploadBackdropImage`**: the fan club banner, coin launch
banner and contest cover uploads each had a copy of the same upload +
largest-variant + node URL code. They now share one helper (with tests).
It reads the node from `sdk.services.storageNodeSelector`, which is the
same instance the old code reached through `(sdk.services.storage as
any).storageNodeSelector`.
- **mobile `useEnterContest`**: if fetching the original track's artwork
failed, tapping "submit a remix" did nothing. It now opens Upload
without the artwork pre-filled.

## Why this tier

These change what happens in a real flow (polling, error rollback, an
upload path, a navigation), but each is confined to one hook or screen
and the sdk is untouched.

## Verified
- `tsc`: common, mobile and web show the same errors as origin/main in
this checkout, all in untouched files.
- `eslint` on changed files: clean.
- common vitest: new `uploadBackdropImage` tests pass. The pre-existing
`store/upload/selectors.test.ts` failure is unchanged.
- mobile jest: all suites pass.

## Not verified
No device or browser testing. Worth a manual check on RC: a USDC
withdrawal on the Pay & Earn page (spinner should stop once the row
appears or after 60s), a fan club banner change and a contest cover
upload.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
dylanjeffers added a commit that referenced this pull request Nov 12, 2026
… selection, relay and chats (#14777)

Fourth of five PRs that re-ship #14665 on current main, ordered from
lowest to highest risk. Branched off main, independent of the others.

## What's in it

sdk error-handling fixes, with a patch changeset, so merging this
publishes an sdk version.

- **Storage uploads**: tus doesn't await `onSuccess`, so a polling
failure became an unhandled rejection and `start()` never settled.
Polling results now go to `resolve`/`reject`.
- **Aborted uploads**: the abort check sat inside the polling `try`, so
the abort error was caught and retried every 3s for up to an hour. It's
checked before the `try`.
- **StorageNodeSelector**: if the first node list fetch failed,
selection ordered the bootstrap nodes and kept that order after a later
fetch succeeded. The ordering is now dropped when the list refreshes,
and selection holds a local reference so a refresh mid-selection can't
swap the list out from under it. New test covers it.
- **Request signature middleware**: after logout or a signing failure it
kept attaching the previous user's cached signature. The cache is
cleared on error. New test covers it.
- **`SolanaRelayWalletAdapter.connect()`**: `connecting` stayed `true`
forever if `getFeePayer` threw.
- **`SolanaRelay.relay()`**: a non-JSON or null error body replaced the
real `ResponseError` with a parse error.
- **`SolanaClient.buildTransaction()`**: the 1.5x priority multiplier
can produce a fractional fee, and `setComputeUnitPrice` throws on it
when converting to `BigInt`. The fee is rounded. New test covers it.
- **ChatsApi**: a rejected public key lookup was cached, which broke
create/invite for that user until reload. Websocket handler rejections
were unhandled; they're now logged.

## Why this tier

Published package, and uploads now reject in cases where they used to
hang. Callers that never handled a rejection from `upload.start()` will
now see one.

## Dropped from #14665
- The constructor-time `updateAvailableStorageNodes` unhandled
rejection: main now fetches the node list lazily with its own catch.
- `isNodeHealthy` getting the sdk logger: skipped here to keep this PR
independent of tier 1, which moves the function.

## Verified
- `tsc` and `eslint` (changed files): clean.
- sdk vitest: all files pass, including the new tests.

## Not verified
No end-to-end upload against a real node.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
dylanjeffers added a commit that referenced this pull request Nov 12, 2026
…enabled guards (#14778)

Fifth of five PRs that re-ship #14665 on current main, ordered from
lowest to highest risk. Branched off main, independent of the others.

## What's in it

Query hooks in common built their options as `{ enabled: guard,
...options }`, so any caller that passed `enabled` replaced the hook's
own guard. For example `useCoinBalance` with `enabled: true` ran the
AUDIO balance queries for non-AUDIO mints, and `useCurrentAccount` could
run without a wallet. These now spread options first and set `enabled:
guard && options?.enabled !== false`, which is what `useUser` already
does.

Hooks changed: `useFanClubs`, `useWalletCoins`, `useUserRemixContests`,
`useCoinExchangeRate`, `usePlaylistUpdates`, `useRemixes`,
`useRemixesCount`, `useCurrentAccount`, `useManagers`, `useCoinBalance`,
`useDestinationUsdcAccountCheck`, `useUSDCBalance`,
`useUserBalanceHistory`, `useMintRecovery`, `useUserbank`.

## Why this tier

Highest blast radius of the set: it touches about 15 widely used hooks.
Any caller that relied on overriding a guard with `enabled: true` will
now see its query stay disabled until the guard passes. I grepped the
callers that pass `enabled` and found only boolean conditions that are
compatible with the guard. One thing to know: TanStack also accepts a
function for `enabled`. These hooks now treat a function as "not false"
and ignore it. No current caller passes one.

## Dropped from #14665
- `useGetStemsArchiveJobStatus`: removed on main with the streamed stems
zip (#14703).

## Verified
- `tsc`: common, mobile and web show the same errors as origin/main in
this checkout, all in untouched files.
- `eslint` on changed files: clean.
- common vitest: same as main (only the pre-existing
`store/upload/selectors.test.ts` failure).

## Not verified
No device or browser testing. Screens most worth a look on RC:
wallet/coin balances, buy/sell, artist coin pages, remix contests and
the account/manager switcher.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@dylanjeffers

Copy link
Copy Markdown
Contributor Author

Split into #14774–#14778, all merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants