feat(Bugs): Implement Bugs! provider#213
Conversation
Bugs! is a South Korean digital streaming service that also offers digital downloads (FLAC or MP3). This provider uses the mobile app's API, which requires an access token that can be obtained from a separate endpoint using the client credentials of the app. The device ID is usually queried from the device the app is running on, but can be any random value as it doesn't seem to be validated. Thus, it's set to a hardcoded value which can optionally be overridden through an environment variable. The brand color and icon have been taken from the website. To implement this change, I used the help of an AI tool (namely Claude Code using the Sonnet 4.6 model), which created the Bugs! API types from the HAR log captured from my device with HTTP Toolkit, and wrote the initial provider implementation and tests scaffold. Regardless, all code changes have been personally reviewed by me, and I take full responsibility for them. I also selected and wrote the test cases myself. Assisted-by: Claude:claude-sonnet-4.6
| headers: { | ||
| 'Content-Type': 'application/json; charset=UTF-8', | ||
| 'Authorization': `Bearer ${accessToken}`, | ||
| 'User-Agent': 'Mobile|Bugs|5.07.00|Android|16|SM-F936B|samsung|market|105070000', |
There was a problem hiding this comment.
We might want to customize this user agent too. In my tests, it wasn't validated, so we could also use Harmony's user agent instead of impersonating the mobile app.
There was a problem hiding this comment.
Have you experimented what happens if we omit the additional headers below? If we were to use Harmony as the user agent, I would like to get rid of those as well.
There was a problem hiding this comment.
I did some more testing and could indeed remove nearly everything. Behaves less like the mobile app now, but still returns the same data.
| private async requestAccessToken(): Promise<ApiAccessToken> { | ||
| const url = new URL('https://secure.bugs.co.kr/api/5/appToken'); | ||
| url.searchParams.set('client_id', 'bugsapp_credentials_android'); | ||
| url.searchParams.set('client_secret', 'd33b!z7xeu'); |
There was a problem hiding this comment.
I'm unsure whether this client ID should be in the source code or not. It's technically public since it's shipped in the APK.
There was a problem hiding this comment.
For Qobuz it is similar with the web app credentials being publicly exposed. The Qobuz provider reads them from environment variables nevertheless, as I feel more comfortable with that approach. So we should do that here as well.
It may be a bit annoying for people who run a Harmony instance having to set those up themselves, but since it only affects specific providers I think this is an acceptable compromise. I haven't made my mind up yet whether I would accept a documentation page which summarizes how to obtain these credentials though.
There was a problem hiding this comment.
Works for me. I'll move the client secret to a config variable then.
kellnerd
left a comment
There was a problem hiding this comment.
Thank you, this is looking good already! I only have a few comments until I get to test the new provider myself.
| override get internalName(): string { | ||
| return 'bugs'; | ||
| } |
There was a problem hiding this comment.
This is not necessary, the base class already implements this normalization.
There was a problem hiding this comment.
I wasn't entirely sure whether it would handle the exclamation mark correctly, but I should have tested that maybe 😅
Works perfectly without, I'll remove this.
| options: releaseOptions, | ||
| assert: async (release, ctx) => { | ||
| await assertSnapshot(ctx, release); | ||
| assert(release.media.length === 1, 'Should have one disc'); |
There was a problem hiding this comment.
Please use assertEquals for the the numeric comparisons, it will show us a diff if those assertions should ever fail. (I know I've also made this mistake myself in some old test cases, but I think I've updated those by now.)
|
|
||
| readonly supportedUrls = new URLPattern({ | ||
| hostname: 'music.bugs.co.kr', | ||
| pathname: String.raw`/:type(album|track|artist)/:id(\d+)`, |
There was a problem hiding this comment.
Do they also have label pages? I'm just asking since those also have numeric IDs which we could then use to resolve them to MB label entities.
| { id: 'album', args: { album_id: albumId, result_type: 'DETAIL' } }, | ||
| { id: 'album_track', args: { album_id: albumId, result_type: 'LIST' } }, | ||
| ]); | ||
| if (bugsDeviceId) apiUrl.searchParams.set('device_id', bugsDeviceId); |
There was a problem hiding this comment.
I haven't tried it yet, but is it necessary to have device_id is as part of the URL? The research in #202 didn't mention it at least.
(It also didn't mention the album_id, but we would need that in any case to distinguish cached API responses by URL.)
There was a problem hiding this comment.
You're right, you can actually omit the device ID. In my testing yesterday, I started with an access token obtained from a logged-in account, where omitting the device ID would throw an error (they probably use it for enforcing device limits for paid subscriptions). However, with the anonymous "account" access token, which appears to be globally unique, the device ID isn't enforced to be supplied since a device limit wouldn't make sense anyway. I'll remove the device ID then.
And yes, the album ID isn't supposed to be part of the URL but is included so that the caching layer works. I'll add a comment explaining this.
| headers: { | ||
| 'Content-Type': 'application/json; charset=UTF-8', | ||
| 'Authorization': `Bearer ${accessToken}`, | ||
| 'User-Agent': 'Mobile|Bugs|5.07.00|Android|16|SM-F936B|samsung|market|105070000', |
There was a problem hiding this comment.
Have you experimented what happens if we omit the additional headers below? If we were to use Harmony as the user agent, I would like to get rid of those as well.
| private async requestAccessToken(): Promise<ApiAccessToken> { | ||
| const url = new URL('https://secure.bugs.co.kr/api/5/appToken'); | ||
| url.searchParams.set('client_id', 'bugsapp_credentials_android'); | ||
| url.searchParams.set('client_secret', 'd33b!z7xeu'); |
There was a problem hiding this comment.
For Qobuz it is similar with the web app credentials being publicly exposed. The Qobuz provider reads them from environment variables nevertheless, as I feel more comfortable with that approach. So we should do that here as well.
It may be a bit annoying for people who run a Harmony instance having to set those up themselves, but since it only affects specific providers I think this is an acceptable compromise. I haven't made my mind up yet whether I would accept a documentation page which summarizes how to obtain these credentials though.
|
|
||
| function mapReleaseType(albumType?: string): ReleaseGroupType[] | undefined { | ||
| if (!albumType) return undefined; | ||
| if (albumType.includes('EP') || albumType.includes('미니')) return ['EP']; |
There was a problem hiding this comment.
Why are you using an includes check here, can the type field contain additional text? From a brief look at the committed testdata, it doesn't seem so.
If it is possible, we should use a simple string lookup mapping which would be more performant as well.
There was a problem hiding this comment.
My main motivation was for hardening here, for example, against white space. But since it is JSON, and the album types generally seem to be stable, I'll replace it with direct comparisons just like I did for the OST type.
|
Thanks for the thorough review! I think I caught & fixed everything. I left your comments open so that you can verify and resolve them yourself. Once the review is complete and before this is merged, I'd like to squash the second commit so that the client secret is removed from the history again. |
Bugs! is a South Korean digital streaming service that also offers digital downloads (FLAC or MP3). This provider uses the mobile app's API, which requires an access token that can be obtained from a separate endpoint using the client credentials of the app. The device ID is usually queried from the device the app is running on, but can be any random value as it doesn't seem to be validated. Thus, it's set to a hardcoded value which can optionally be overridden through an environment variable. The brand color and icon have been taken from the website.
To implement this change, I used the help of an AI tool (namely Claude Code using the Sonnet 4.6 model), which created the Bugs! API types from the HAR log captured from my device with HTTP Toolkit, and wrote the initial provider implementation and tests scaffold. Regardless, all code changes have been personally reviewed by me, and I take full responsibility for them. I also selected and wrote the test cases myself.
Inspired by and closes #202.