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.

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

Crash when refreshing releasing groups and there is a new release group #859

Closed
lydavid opened this issue Apr 8, 2024 · 3 comments · Fixed by #929
Closed

Crash when refreshing releasing groups and there is a new release group #859

lydavid opened this issue Apr 8, 2024 · 3 comments · Fixed by #929
Labels
bug Something isn't working

Comments

@lydavid
Copy link
Owner

lydavid commented Apr 8, 2024

Or it might be because a release group was removed. Need to find a way to test for each of these scenarios

@lydavid lydavid added the bug Something isn't working label Apr 8, 2024
@lydavid
Copy link
Owner Author

lydavid commented Apr 22, 2024

Tested with Charles by rewriting with an additional release group, then refreshing. I could not reproduce it.
There may have been a bad migration at some point.

@lydavid lydavid closed this as completed Apr 22, 2024
@lydavid lydavid reopened this May 25, 2024
@lydavid
Copy link
Owner Author

lydavid commented May 25, 2024

I was able to repro on debug app.

Let's make sure to clear total remote count, and refetch it when refreshing.

@lydavid
Copy link
Owner Author

lydavid commented May 26, 2024

Steps to track down now that I can repro on debug build

  • build to phone
  • scroll and crash to check logcat

release-group inside artist screen causing crash:
error: Key "e289b05a-9094-4a17-956f-04bffbc09427" was already used.

select *
from release_group
where id = "e289b05a-9094-4a17-956f-04bffbc09427";

only gives me one result cause it's a primary key

Run getNumberOfReleaseGroupsByArtist from AS's gutter gives me 64 for artist 9df7ddfb-75fe-4470-9269-5810d9263b09 whereas getBrowseEntityCount gives 62 for both local and remote count.

Run getReleaseGroupsByArtist to get the 64 release groups.
Modify with and rg.id = "e289b05a-9094-4a17-956f-04bffbc09427" to find the problem rg.
They look the same.
Getting rid of LEFT JOIN mbid_image mi ON mi.mbid = rg.id still gives 2 result.
Querying for the artist or rg from artist_release_group gives just 1 result.

Problem is:

INNER JOIN artist_credit_entity acr ON acr.entity_id = rg.id
INNER JOIN artist_credit ac ON ac.id = acr.artist_credit_id

removing this will give just 1 result

Selecting acr.* shows the problem:
This artist has artist_credit_id 10 and 15 (ids chosen by autoincrement).

Fixes

The simplest fix is to select DISTINCT.

But I need to prevent duplicates showing up in artist_credit.
Looking into artist_credit, I think I found the actual issue. It is because this artist is entered into this table as both HOYO-MiX and HOYO‐MiX, where the only difference is in the dash.
The first one is: https://apps.timwhitlock.info/unicode/inspect?s=-
The second one is: https://apps.timwhitlock.info/unicode/inspect?s=%E2%80%90
artist_credit already checks to ensure we do not insert duplicate names, but cannot possibly check this different unique of unicode characters.
So, in the end, I will just select DISTINCT and not deal with this unique scenario.

SELECT DISTINCT doesn't work when I need it to be distinct on just one column (rg.id): https://stackoverflow.com/a/5585483

Afterthoughts

This issue actually came up because of an artist credit name change for a release group upstream after refreshing. Any changes in Unicode counts as a new artist according to our app. I thought about deleting the artist credits as well, but decided against it because other entities may still use the artist credit name. We just need to accept that there may exist some erroneous artist credit names in the app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant