[CON-222] Fix SequelizeUniqueConstraintError for blockchain track ID #3660

theoilie · 2022-08-09T02:25:39Z

Description

The following error is caused by load test on staging, so it doesn't impact prod (verify lack of prod impact by searching the following in loggly with and without the audius-stage/prod tag: "SequelizeUniqueConstraintError" tag:"audius-prod" -"Queue.onFailed" -"secondarySyncFromPrimary failure for wallets" "Tracks_unique"): [SequelizeUniqueConstraintError] blockchainId must be unique
blockchainId is a smart-contract generated global track ID that is supposed to be unique
Likely something with the load test uploading multiple tracks at the same time caused it to make Content Node's /tracks endpoint associate the same track ID with multiple uploads
The solution to prevent this broken state from being created in the future is to make Content Node's /tracks endpoint verify that the owner of the track ID on chain is the same owner of the wallet attempting to associate the ID with their track
This PR also applies the same solution for the /audius_users endpoint to prevent a similar issue in the AudiusUsers table

Tests

Updated integration tests
Added new test to make sure /tracks errors when it's given a wallet+trackId that doesn't match what's on-chain

Monitoring - How will this change be monitored? Are there sufficient logs / alerts?

We'll need to fix the currently broken state before verifying that no more SequelizeUniqueConstraintError logs happen in prod due to the blockchainId+clock unique constraint
Watch track uploads carefully on staging to make sure there are no new failures -- particularly with the error message does not match the ID of the user attempting to write this track

vicky-g · 2022-08-09T04:50:23Z

creator-node/src/routes/tracks.js

+        const trackResp = await libs.contracts.TrackFactory.getTrack(
+          blockchainTrackId
+        )
+        // eslint-disable-next-line eqeqeq


is this a real eslint one-liner lol

yep it is https://eslint.org/docs/latest/rules/eqeqeq. but the real question is why do we need it?

one more thing - can we add this same check to POST /audius_users? sorry i know this is expanding the scope of this PR but might be worth having that safety

I see a similar SequelizeUniqueConstraintError logged for the AudiusUsers table, so adding another check should prevent that as well. I just pushed this out now. also, see my comment below for why this is using eqeq instead of eqeqeq

vicky-g

vicky-g · 2022-08-09T04:52:16Z

but lets fix mad dog 😢 https://app.circleci.com/pipelines/github/AudiusProject/audius-protocol/26240/workflows/4286345b-e6b1-4fa5-bf19-d2a6047db99f/jobs/313479?invite=true#step-101-860

vicky-g

only bc mad dog is failing with a track related err

dmanjunath

can't believe we never had it before. one question about why not !== for the equality comparison but otherwise this looks great, thanks for adding this check.

definitely should fix mad dog as well, but i feel like rebasing to latest master should fix that. mad dog seems to be passing on there

dmanjunath · 2022-08-09T11:18:20Z

creator-node/src/routes/tracks.js

+        const trackResp = await libs.contracts.TrackFactory.getTrack(
+          blockchainTrackId
+        )
+        // eslint-disable-next-line eqeqeq


yep it is https://eslint.org/docs/latest/rules/eqeqeq. but the real question is why do we need it?

theoilie · 2022-08-09T16:18:26Z

can't believe we never had it before. one question about why not !== for the equality comparison but otherwise this looks great, thanks for adding this check.

definitely should fix mad dog as well, but i feel like rebasing to latest master should fix that. mad dog seems to be passing on there

thank you both for reviewing -- I'll rebase shortly. I used eqeq instead of eqeqeq because sometimes one side could be a string and the other could be a number. I'm not sure if that was only caused by something in the integration tests, but I figured if it's happening in tests then I should account for it in prod as well

theoilie · 2022-08-10T01:49:15Z

got tests + maddog passing with the requested /audius_users change and updated PR description to reflect that

dmanjunath

Thanks for adding the audius_users check! Two small comments but this looks awesome

creator-node/src/routes/tracks.js

creator-node/test/sync.test.js

theoilie · 2022-08-10T17:23:14Z

dismissing your review @vicky-g so I can merge since it was for mad dog and mad dog is now passing

was for mad dog, which is now passing

## Changelog - 2022-08-11 [8c2deb9] Use correct configuration for full endpoints (#3673) [Sebastian Klingler] - 2022-08-10 [5ab2d47] [CON-222] Fix SequelizeUniqueConstraintError for blockchain track+user IDs (#3660) [Theo Ilie] - 2022-08-10 [1f55962] Bump to version 0.3.64 (#3669) [Cheran] - 2022-08-10 [8e21b0e] Bump random replica select attempt count + filter out existing nodes from healthy replica set (#3661) [vicky :)] - 2022-08-09 [19da757] Fix TypeError for userSecondarySyncMetrics (#3668) [Theo Ilie] - 2022-08-09 [788d482] CON-297 Auth check the requester is the primary of the observed user (#3598) [vicky :)] - 2022-08-09 [7436c17] Add unique constraint to notification (#3657) [Joseph Lee] - 2022-08-09 [4d03415] Add slack-secrets to jobs that use Slack notifications (#3666) [Joaquin Casares] - 2022-08-09 [feed5ab] Revert "Repair missing wallets (#3656)" (#3659) [Raymond Jacobson] - 2022-08-09 [8196748] Change openresty location directive from exact match to regex (#3664) [Dheeraj Manjunath] - 2022-08-08 [1c2f069] Repair missing wallets (#3656) [Raymond Jacobson] - 2022-08-08 [f4d5481] Upgrade service-commands to latest sdk (#3655) [Dheeraj Manjunath] - 2022-08-08 [90a1601] safer kill_running_queries_sql (#3653) [Steve Perkins] - 2022-08-08 [6194260] Rename processImmediateSync to processManualImmediateSync for clarity (#3632) [Dheeraj Manjunath] - 2022-08-05 [f25994c] Fix Prometheus Metric Names for Bull Queue Durations (#3649) [Johannes Naylor] - 2022-08-05 [a2d9937] Add remix cosign (#3650) [Joseph Lee] - 2022-08-05 [bbf5556] [C-749] Support req pagination and ordering in hist/fav/tracks (#3643) [Raymond Jacobson] - 2022-08-05 [d6acc56] INF add alerts for failed gcp bake jobs (#3647) [Joaquin Casares] - 2022-08-05 [a0ee00a] postgres triggers for discovery notifications (#3464) [Joseph Lee] - 2022-08-05 [7a4d9ea] Bump sdk to v0.0.32 [audius-infra]

…r IDs (#3660)

[4a26077] [C-2678] Add Stems and Source Files Modal (#3671) Andrew Mendelsohn [6a3342a] Bump android version to 1.1.391 to fix play-store build (#3678) Dylan Jeffers [2064212] [C-2813] Catch collectibles runtime errors (#3675) Dylan Jeffers [29233ea] [C-2808] Improve storageNodeSelector usage and perf (#3674) Dylan Jeffers [2de2035] Fix mobile image uri (#3670) Dylan Jeffers [55a6089] [C-2807] Add ModalField subforms with cancellation (#3664) Andrew Mendelsohn [e26986f] [C-2812] Use storageNodeSelector in libs, fix mobile images (#3667) Dylan Jeffers [5daea08] Enable playlist updates on prod (#3668) Dylan Jeffers [d90c369] [PAY-1541] Autofocus textinput on mobile search users screen (#3666) Reed [194f894] [PAY-1542][PAY-1539][PAY-1537] Mobile chats QA improvements (#3665) Reed [58b9028] [C-2798] Fix playlist button overlap (#3662) Dylan Jeffers [a935588] [PAY-1538] Fix autocorrect behavior (#3661) Michael Piazza [1a8a6e7] [PAY-1518] DMs: Align the overflow menu bottom center (#3654) Marcus Pasell [c880b2a] [plat-1085] disable fav and repost on hidden tracks in play bar (#3653) sabrina-kiam [d80617e] [PAY-1529] Add initial types for usdc purchases (#3651) Randy Schott [68e87ff] [C-2787] Reregister device-token on app startup (#3660) Dylan Jeffers [ddfa510] [C-2802] Move embed to client monorepo! (#3659) Raymond Jacobson [6a2f008] Add Sentry error logging to Audius query C-2800 (#3656) nicoback2 [79213de] Add Amplitude events to new Write OAuth flows C-2801 (#3657) nicoback2 [6576ae4] Fix android chat message cut off at top (#3658) Reed [160b325] [C-2677] Remix Settings Modal layout complete (#3647) Andrew Mendelsohn [c01c022] [C-2790] Add private DogEar to desktop playlist cards (#3655) Dylan Jeffers [50e8f4e] [PAY-1522][PAY-1451] Fix issues with DMs notification dot (#3649) Michael Piazza [1658810] [PAY-1530] adds usdc feature flag and hooks for fetching it (#3645) Randy Schott

theoilie added 2 commits August 9, 2022 02:11

Fix SequelizeUniqueConstraintError for blockchain track ID

c998e61

Add test to assert failure on track ID mismatch

5a8ccd2

theoilie requested review from dmanjunath and vicky-g August 9, 2022 02:25

theoilie assigned SidSethi Aug 9, 2022

pull-request-size bot added the size/L label Aug 9, 2022

theoilie added the content-node Content Node (previously known as Creator Node) label Aug 9, 2022

vicky-g reviewed Aug 9, 2022

View reviewed changes

vicky-g approved these changes Aug 9, 2022

View reviewed changes

vicky-g previously requested changes Aug 9, 2022

View reviewed changes

dmanjunath approved these changes Aug 9, 2022

View reviewed changes

theoilie added 5 commits August 9, 2022 16:27

Merge remote-tracking branch 'origin' into theo-con222-sequelize

0d05b69

Fix TrackFactoryClient being referenced as TrackFactory

ad2a6d0

Add validation to /audius_users route

53ce61b

Fix missed renames

18fd5f8

Fix string case comparison

a4863f7

theoilie requested review from vicky-g and dmanjunath August 10, 2022 01:46

dmanjunath approved these changes Aug 10, 2022

View reviewed changes

creator-node/src/routes/tracks.js Outdated Show resolved Hide resolved

creator-node/test/sync.test.js Outdated Show resolved Hide resolved

Remove only and use strict equality check

9fe065a

theoilie merged commit 5ab2d47 into master Aug 10, 2022

theoilie deleted the theo-con222-sequelize branch August 10, 2022 17:23

dmanjunath pushed a commit that referenced this pull request Aug 16, 2022

[CON-222] Fix SequelizeUniqueConstraintError for blockchain track+use…

9f5d7de

…r IDs (#3660)

sliptype pushed a commit that referenced this pull request Sep 10, 2023

[C-2787] Reregister device-token on app startup (#3660)

b06caa6

AudiusProject deleted a comment from linear bot Sep 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CON-222] Fix SequelizeUniqueConstraintError for blockchain track ID #3660

[CON-222] Fix SequelizeUniqueConstraintError for blockchain track ID #3660

theoilie commented Aug 9, 2022 •

edited

Loading

vicky-g Aug 9, 2022 •

edited

Loading

dmanjunath Aug 9, 2022

dmanjunath Aug 9, 2022

theoilie Aug 10, 2022

vicky-g left a comment

vicky-g commented Aug 9, 2022

vicky-g left a comment

dmanjunath left a comment •

edited

Loading

dmanjunath Aug 9, 2022

theoilie commented Aug 9, 2022

theoilie commented Aug 10, 2022

dmanjunath left a comment

theoilie commented Aug 10, 2022

[CON-222] Fix SequelizeUniqueConstraintError for blockchain track ID #3660

[CON-222] Fix SequelizeUniqueConstraintError for blockchain track ID #3660

Conversation

theoilie commented Aug 9, 2022 • edited Loading

Description

Tests

Monitoring - How will this change be monitored? Are there sufficient logs / alerts?

vicky-g Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

dmanjunath Aug 9, 2022

Choose a reason for hiding this comment

dmanjunath Aug 9, 2022

Choose a reason for hiding this comment

theoilie Aug 10, 2022

Choose a reason for hiding this comment

vicky-g left a comment

Choose a reason for hiding this comment

vicky-g commented Aug 9, 2022

vicky-g left a comment

Choose a reason for hiding this comment

dmanjunath left a comment • edited Loading

Choose a reason for hiding this comment

dmanjunath Aug 9, 2022

Choose a reason for hiding this comment

theoilie commented Aug 9, 2022

theoilie commented Aug 10, 2022

dmanjunath left a comment

Choose a reason for hiding this comment

theoilie commented Aug 10, 2022

theoilie commented Aug 9, 2022 •

edited

Loading

vicky-g Aug 9, 2022 •

edited

Loading

dmanjunath left a comment •

edited

Loading