Repository navigation
fix: tech-debt pass — leaked listeners, async error handling, query guards, dead code - #14665
dylanjeffers wants to merge 6 commits into
Conversation
…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
…code 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 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
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- 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
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 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 detectedLatest commit: 8449b61 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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
|
GitGuardian finding 32408190 ( 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
…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>
…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>
…, 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>
… 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>
…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>
Code review and tech-debt pass over
sdk,harmony,common,mobileandweb. There is one commit per package.sdk (patch changeset:
.changeset/sdk-error-handling-fixes.md)onSuccess, so a polling failure became an unhandled rejection andstart()never settled./health_checkcaused an unhandled rejection, which kills a Node process.connectingstayedtrueforever ifgetFeePayerthrew.ResponseError.BigInt()throw a RangeError, so the fee is now rounded.generatePreviewnow throwsNo validator node available.harmony
Modal: thetouchmovelistener leaked on unmount and stacked up whenonTouchMoveormodalRootchanged.Slider: document listeners leaked when it unmounted mid-drag.FilterButtonKeyHandler;findAncestornow usesclosest().common
enabled: guard, ...optionslet a caller's{ enabled: true }drop the hook's own guard in about 15 hooks.useCurrentAccountran with an undefined wallet, anduseCoinBalancefired the AUDIO queries for non-AUDIO mints.enabled: guard && options.enabled !== false, matchinguseUser.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 isNaN & 3, so it was biased.uploadBackdropImagehelper, with tests.IOS_USDC_PURCHASE_ENABLED,REACT_QUERY_SYNC,COLLAPSED_EXPLORE_HEADER.@ts-ignores with proper types.mobile
PlaybackActiveTrackChangedlistener was never removed, so it piled up per reconnect or track change and kept calling stale cast clients..stop(), so the listener was never removed.useRevertOnCancel: thebeforeRemovelistener was never unsubscribed.audius-sdk: every concurrent init waiter added a permanent listener; it now usesonce().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.@ts-ignores.web
WithdrawalsTab: the withdrawal poller never stopped.reset(new identity every render) andtransactionswere effect deps, so the effect restarted on every render. That also restarted the 60s timeout, and if the transaction never appeared it calledresetQueriesevery second forever.StripeOnRampModal:onramp_session_updatedlistener was never removed, so it stacked up on each reopen with the same session and dispatched status changes more than once.foregroundPollingDaemon: thevisibilitychangelistener is now removed when the saga is cancelled.useScript, fixed the hooks README, and updated terminology.Found but not fixed
useHotKeys: builds a newthrottle()on every keydown, so it never throttles. This is public API behavior, so it's left alone.useModalRoot: a modal unmounted while open leaves its root, container and backdrop divs indocument.body.useUSDCTransactions:queryKeyis rebuilt every render, soresetis unstable for every consumer. Memoizing the key would fix it at the source.SearchExploreScreen: an effect with no deps array scrolls to the top on every render while the query is one character or less.usePlaybackPositionPersistence:JSON.parsehas no try/catch.true(USDC_PURCHASES,LAUNCHPAD_VERIFICATION,WEEKLY_ROTATION): their off-branches are left in place.keyboardredux slice.web/src/assets, including all ofassets/fonts.AnimatedIconButton's dynamicimport()makes Vite emit a chunk for every JSON in the animations folder, so the unused animations also add build output.commonstore/upload/selectors.test.tswas already failing (7 failures); its files aren't touched here.Testing
tsc: 0 errors.eslint: clean.vitest: 216 passed, 2 skipped.StorageNodeSelectortest needsNO_PROXYin the sandbox because axios goes through the proxy; that's unrelated to this PR.tscandeslintclean.tscandeslintclean.vitest: 30 of 31 files pass; the one failure is the pre-existing file above.tsc: 0 errors.eslint: same result as before this PR.jest: 34 of 34 pass.tsc: 0 errors.eslint: clean on changed files.🤖 Generated with Claude Code
https://claude.ai/code/session_01J37cvyRSg4MeLrVoQA1Uhh