Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[PAY-951] SDK: Allow chat messages to have externally specified IDs, gracefully fail decryption #4963

Merged
merged 3 commits into from Mar 21, 2023
Merged

[PAY-951] SDK: Allow chat messages to have externally specified IDs, gracefully fail decryption #4963

merged 3 commits into from Mar 21, 2023

Conversation

rickyrombo
Copy link
Contributor

@rickyrombo rickyrombo commented Mar 17, 2023 •
edited

Description

  • Catch errors when decrypting and gracefully exit
  • Allow consumers to specify chat message IDs so that they can optimistically update their UIs.
    • Not in love with this.. would much prefer the SDK be in charge of how these IDs get generated. However, I'm not sure what a better solution is?
  • Use ulid instead of cuid

Tests

Tested by linking libs to a Client PR and throwing fake errors in the decrypt method

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

@rickyrombo rickyrombo mentioned this pull request Mar 21, 2023
Merged
rickyrombo
rickyrombo commented Mar 21, 2023
View reviewed changes
libs/src/sdk/api/chats/ChatsApi.ts
)
} catch (e) {
console.error(
"[audius-sdk]: Error: Couldn't decrypt last chat message",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't think of a great way to capture this error in a way that the client could handle it more gracefully and still have the chats/messages that succeeded load

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this seems like a good option... hopefully it's a rare thing.

stereosteve
stereosteve approved these changes Mar 21, 2023
View reviewed changes
Copy link
Contributor

@stereosteve stereosteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

libs/src/sdk/api/chats/ChatsApi.ts
)
} catch (e) {
console.error(
"[audius-sdk]: Error: Couldn't decrypt last chat message",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this seems like a good option... hopefully it's a rare thing.

@rickyrombo rickyrombo merged commit a5505e4 into main Mar 21, 2023
1 check passed
@rickyrombo rickyrombo deleted the mjp-dms-message-ids branch March 21, 2023 18:44
@rickyrombo rickyrombo mentioned this pull request Mar 21, 2023
Merged
isaacsolo added a commit that referenced this pull request Mar 24, 2023
@isaacsolo
Revert "[PAY-951] SDK: Allow chat messages to have externally specifi…
730190d 
…ed IDs, gracefully fail decryption (#4963)"

This reverts commit a5505e4.
audius-infra pushed a commit that referenced this pull request Mar 24, 2023
Bump sdk to v2.0.3-beta.1
576cbaa 
## Changelog

- 2023-03-24 [b87aba3] Fix unclaimed ID caching for safari (#5002) [Isaac Solo]
- 2023-03-22 [490d54d] Comms user chat  permissions endpoints (#4966) [Michelle Brier]
- 2023-03-21 [6b07209] [PLAT-789] Remove anchor audius data (#3743) [Raymond Jacobson]
- 2023-03-21 [bc1c9b6] Bump SDK to v2.0.3-beta.0 (#4983) [Marcus Pasell]
- 2023-03-21 [a5505e4] [PAY-951] SDK: Allow chat messages to have externally specified IDs, gracefully fail decryption (#4963) [Marcus Pasell]
- 2023-03-16 [186f483] SDK 2.0.2 (#4949) [Michael Piazza]
- 2023-03-16 [0ab0604] [PAY-851] Address blockhash not found error (#4939) [Michael Piazza]
- 2023-03-14 [640fbe2] SDK: Retry once with new selections on failed requests (#4773) [Marcus Pasell]
- 2023-03-08 [1a4d4e8] Bump sdk to v2.0.2-alpha.4 (#4905) [Dylan Jeffers]
- 2023-03-08 [a8d6b1b] [C-2268] Add missing valid_types params to get-notifications endpoint (#4900) [Dylan Jeffers]
- 2023-03-07 [9972e6e] Add min followers to feeling lucky query (#4883) [Michelle Brier]
- 2023-02-23 [0c6e914] Revert "Revert CI changes" (#4812) [Cheran]
- 2023-02-21 [6fadb50] Bump SDK (#4808) [Michael Piazza]
- 2023-02-21 [94ac114] lint libs notifications (#4807) [Joseph Lee]
- 2023-02-21 [2ef3859] [PAY-902] Support adhoc userbank creation (#4795) [Michael Piazza]
- 2023-02-21 [7dae8e2] Fix notifications endpoint (#4769) [Joseph Lee]
- 2023-02-16 [975031c] SDK: Create chat IDs deterministically (#4788) [Marcus Pasell]
- 2023-02-15 [3fb92a6] Revert CI changes (#4778) [Raymond Jacobson]
- 2023-02-15 [41d9be3] Update entity manager libs social actions to accept metadata (#4771) [sabrina-kiam]
- 2023-02-14 [1673f43] SDK v2.0.2-alpha.1 (#4766) [Marcus Pasell]
- 2023-02-13 [bb18129] SDK: Better listener types for Chats, DN Selector (#4757) [Marcus Pasell]
- 2023-02-13 [00262f7] Replace circle config (#4553) [Cheran]
- 2023-02-11 [673f547] Bump sdk to v2.0.1 [audius-infra]
audius-infra pushed a commit that referenced this pull request Mar 24, 2023
Bump sdk to v2.0.3-beta.1
91fc6e2 
## Changelog

- 2023-03-24 [b87aba3] Fix unclaimed ID caching for safari (#5002) [Isaac Solo]
- 2023-03-22 [490d54d] Comms user chat  permissions endpoints (#4966) [Michelle Brier]
- 2023-03-21 [6b07209] [PLAT-789] Remove anchor audius data (#3743) [Raymond Jacobson]
- 2023-03-21 [bc1c9b6] Bump SDK to v2.0.3-beta.0 (#4983) [Marcus Pasell]
- 2023-03-21 [a5505e4] [PAY-951] SDK: Allow chat messages to have externally specified IDs, gracefully fail decryption (#4963) [Marcus Pasell]
- 2023-03-16 [186f483] SDK 2.0.2 (#4949) [Michael Piazza]
- 2023-03-16 [0ab0604] [PAY-851] Address blockhash not found error (#4939) [Michael Piazza]
- 2023-03-14 [640fbe2] SDK: Retry once with new selections on failed requests (#4773) [Marcus Pasell]
- 2023-03-08 [1a4d4e8] Bump sdk to v2.0.2-alpha.4 (#4905) [Dylan Jeffers]
- 2023-03-08 [a8d6b1b] [C-2268] Add missing valid_types params to get-notifications endpoint (#4900) [Dylan Jeffers]
- 2023-03-07 [9972e6e] Add min followers to feeling lucky query (#4883) [Michelle Brier]
- 2023-02-23 [0c6e914] Revert "Revert CI changes" (#4812) [Cheran]
- 2023-02-21 [6fadb50] Bump SDK (#4808) [Michael Piazza]
- 2023-02-21 [94ac114] lint libs notifications (#4807) [Joseph Lee]
- 2023-02-21 [2ef3859] [PAY-902] Support adhoc userbank creation (#4795) [Michael Piazza]
- 2023-02-21 [7dae8e2] Fix notifications endpoint (#4769) [Joseph Lee]
- 2023-02-16 [975031c] SDK: Create chat IDs deterministically (#4788) [Marcus Pasell]
- 2023-02-15 [3fb92a6] Revert CI changes (#4778) [Raymond Jacobson]
- 2023-02-15 [41d9be3] Update entity manager libs social actions to accept metadata (#4771) [sabrina-kiam]
- 2023-02-14 [1673f43] SDK v2.0.2-alpha.1 (#4766) [Marcus Pasell]
- 2023-02-13 [bb18129] SDK: Better listener types for Chats, DN Selector (#4757) [Marcus Pasell]
- 2023-02-13 [00262f7] Replace circle config (#4553) [Cheran]
- 2023-02-11 [673f547] Bump sdk to v2.0.1 [audius-infra]
@AudiusProject AudiusProject deleted a comment from linear bot Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stereosteve stereosteve stereosteve approved these changes

Assignees

@dharit-tan dharit-tan

@piazzatron piazzatron

@sddioulde sddioulde

Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@rickyrombo @stereosteve @dharit-tan @piazzatron @sddioulde