Conversation
|
@Xaxxoo is attempting to deploy a commit to the olufunbiik's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR introduces a complete waveform generation system with BullMQ job queuing, TypeORM persistence, and HTTP endpoints for tracks. It replaces in-process retry timers with a durable background job processor, enabling recovery after process restarts and clearer status tracking. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Controller as WaveformController
participant Service as WaveformService
participant Queue as BullMQ Queue
participant Processor as WaveformProcessor
participant Generator as WaveformGeneratorService
participant DB as Database
Client->>Controller: POST /tracks/{trackId}/waveform/regenerate
Controller->>Service: regenerate(trackId)
Service->>DB: Check if waveform record exists
DB-->>Service: WaveformEntity
Service->>DB: Reset status to PENDING, clear failReason
DB-->>Service: Updated
Service->>Queue: Enqueue job {trackId, audioFilePath}
Queue-->>Service: Job ID
Service->>DB: Persist bullJobId
DB-->>Service: Saved
Service-->>Controller: {result: 'queued', jobId}
Controller-->>Client: 200 OK
Queue->>Processor: Dequeue job
Processor->>Service: markProcessing(trackId)
Service->>DB: Set status = PROCESSING
Processor->>Generator: generateFromFile(audioFilePath)
Generator-->>Processor: {peaks, durationSeconds}
Processor->>Service: markDone(trackId, peaks)
Service->>DB: Set status = DONE, save peaks
DB-->>Service: Updated
Processor-->>Queue: Job complete
alt On Generation Failure
Generator-->>Processor: Error
Processor->>Service: markFailed(trackId, reason, attempts)
Service->>DB: Set status = FAILED, failReason
DB-->>Service: Updated
Processor-->>Queue: Rethrow (triggers backoff/retry)
end
Client->>Controller: GET /tracks/{trackId}/waveform
Controller->>Service: getStatus(trackId)
Service->>DB: Fetch WaveformEntity
DB-->>Service: Entity with status, peaks, attemptCount, updatedAt
Service-->>Controller: WaveformStatusDto
Controller-->>Client: 200 OK {status, peaks, attempts, updatedAt}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
backend/src/mount-waveform/waveform-generator.service.ts (2)
5-9:durationSecondsis computed but never persisted or returned to clients.Based on the related code,
WaveformEntityhas nodurationSecondscolumn,markDoneonly persistspeaks, andWaveformStatusDtodoesn't include duration. If duration is intentionally deferred, consider removing it fromGeneratedWaveformto avoid confusion. If it's needed, the entity and service need corresponding updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/mount-waveform/waveform-generator.service.ts` around lines 5 - 9, The GeneratedWaveform.durationSeconds field is computed but never persisted or returned; either remove it from GeneratedWaveform to avoid confusion or persist and surface it: add a durationSeconds column to WaveformEntity, update the markDone method in the waveform-generator service to store generated.durationSeconds when saving peaks, and include durationSeconds in WaveformStatusDto (and any response mapping) so clients receive the value.
30-42: Synchronous filesystem calls block the event loop.
fs.existsSyncandfs.statSyncare synchronous operations that block the Node.js event loop. In a request/job processing context, this can degrade throughput under load. Since this is an async method, prefer async alternatives.♻️ Use async fs operations
- if (!fs.existsSync(filePath)) { - throw new Error(`Audio file not found: ${filePath}`); - } - - // ------------------------------------------------------------------ - // Real implementation would shell out to `audiowaveform` or `ffprobe`. - // The stub below produces deterministic fake data so the rest of the - // stack (queue, persistence, API) can be exercised in tests. - // ------------------------------------------------------------------ - const stats = fs.statSync(filePath); + const fsp = fs.promises; + let stats: fs.Stats; + try { + stats = await fsp.stat(filePath); + } catch { + throw new Error(`Audio file not found: ${filePath}`); + } + + // ------------------------------------------------------------------ + // Real implementation would shell out to `audiowaveform` or `ffprobe`. + // The stub below produces deterministic fake data so the rest of the + // stack (queue, persistence, API) can be exercised in tests. + // ------------------------------------------------------------------🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/mount-waveform/waveform-generator.service.ts` around lines 30 - 42, Replace the synchronous fs calls with their async equivalents in the async method that contains this code: use await fs.promises.access(filePath, fs.constants.R_OK) (or try/catch around await fs.promises.stat(filePath)) to check existence and then await fs.promises.stat(filePath) for stats; throw the same Error if access/stat fails. Ensure peakCount calculation still uses stats.size and keep calls to this.buildFakePeaks(peakCount, filePath) unchanged, and wrap the await calls in try/catch to propagate the existing error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/mount-waveform/tracks.service.spec.ts`:
- Around line 4-6: The import for TrackEntity in tracks.service.spec.ts is
pointing to a non-existent './track.entity'; update the import to reference the
actual module location by changing the TrackEntity import to the correct
relative path (replace the './track.entity' import with the path leading to
tracks/entities/track.entity), so the TracksService tests can resolve
TrackEntity correctly; ensure the import line importing TrackEntity matches the
module export name and compile.
In `@backend/src/mount-waveform/tracks.service.ts`:
- Around line 9-11: The imports for WaveformService, CreateTrackDto and
TrackEntity are pointing to the wrong module locations; update the import
statements so WaveformService and any waveform-related types are imported from
the mount-waveform module where the waveform code actually lives and ensure
TrackEntity is imported from the module that defines the entity (replace the
incorrect '../waveform/waveform.service' and './track.entity' imports with the
correct module locations used elsewhere in this PR so the spec/build can resolve
WaveformService, CreateTrackDto and TrackEntity).
- Around line 33-43: In createTrack (tracks.service.ts) the code passes a
non-existent field track.audioFilePath to waveformService.enqueueForTrack
causing undefined to be enqueued; change the argument to use the Track entity's
actual field track.audioUrl (i.e., call enqueueForTrack(track.id,
track.audioUrl)), keeping the rest of the createTrack logic and logger the same
so waveformService.enqueueForTrack receives the correct URL value.
In `@backend/src/mount-waveform/waveform.controller.spec.ts`:
- Line 3: The test import for WaveformStatus in waveform.controller.spec.ts
fails because the DTO module is missing or the relative path is incorrect;
either add the missing DTO file exporting WaveformStatus (e.g., export enum
WaveformStatus { ... } or export type/const as used) or correct the import path
to the actual location of the DTO, then ensure the exported symbol name matches
the import (WaveformStatus) so TypeScript can resolve './dto/waveform.dto' in
the test.
In `@backend/src/mount-waveform/waveform.controller.ts`:
- Around line 47-56: The Swagger decorator `@ApiConflictResponse` documents a 409
but the handler always returns 200 with { result: 'already_processing' }; update
the runtime to match the contract or update the docs: either (A) change the busy
branch in the controller method (the method that returns RegenerateResponseDto /
currently returns { result: 'already_processing' }) to throw new
ConflictException('Waveform generation already in progress') so the endpoint
actually emits a 409, or (B) remove the `@ApiConflictResponse` decorator and any
mention of a 409 so the OpenAPI spec matches the current behavior; locate
references to HttpCode(HttpStatus.OK), RegenerateResponseDto and the handler
method in waveform.controller.ts to make the change.
In `@backend/src/mount-waveform/waveform.module.ts`:
- Line 9: The import of WaveformProcessor in waveform.module.ts is wrong; change
the module's import for the symbol WaveformProcessor to reference the
parent-module location (use the parent-relative path so it resolves to the file
that exports WaveformProcessor). Also open waveform.processor.ts and correct its
broken imports: update the imports for WaveformGeneratorService,
WaveformService, and any waveform constants so they point to the actual
locations of those symbols (adjust the relative paths so the imports resolve
from the processor file's location). Ensure all three affected symbols
(WaveformProcessor, WaveformGeneratorService, WaveformService, and waveform
constants) import using correct relative paths so the TypeScript compiler can
resolve them.
In `@backend/src/mount-waveform/waveform.service.ts`:
- Around line 56-66: The short-circuit in enqueueForTrack that returns when
record.status is PENDING/PROCESSING can permanently orphan tracks because the DB
row is written before the BullMQ job is created; update enqueueForTrack to
verify the presence and liveness of record.bullJobId (and/or the corresponding
job in the queue) before returning, or invert the handoff so you create the
BullMQ job first and then persist record.bullJobId in the DB (or perform both
operations inside a transaction/atomic upsert). Concretely, change the logic
around the check of record.status / record.bullJobId so that if status is
PENDING/PROCESSING you still confirm record.bullJobId is non-empty and the job
exists in the queue (or re-enqueue and update DB) rather than immediately
returning '', and when creating a new job call queue.add() first (or use a DB
transaction) and persist the returned job id into record.bullJobId to avoid the
race.
- Around line 127-140: regenerate() falls back to the placeholder
__RESOLVE_FROM_TRACK__${trackId} when the BullMQ job was removed
(removeOnComplete: true), which results in waveform-generator.generateFromFile()
getting an invalid path and throwing; update the logic so the real audioFilePath
is available when queuing a regeneration: either persist audioFilePath to the
waveform record when you first enqueue (save the value used in enqueueForTrack)
and read it here (replace the placeholder), or, if you prefer not to store it,
resolve the path from the tracks table before calling enqueueForTrack; change
the code paths involving record.bullJobId, waveformQueue.getJob,
WaveformJobPayload, audioFilePath and enqueueForTrack to prefer the
persisted/resolved audioFilePath over the __RESOLVE_FROM_TRACK__ placeholder.
- Around line 81-93: The enqueue step silently fails when a previous failed job
with jobId `waveform:${trackId}` still exists (removeOnFail: false); update the
enqueue logic (where waveformQueue.add is called for WAVEFORM_JOBS.GENERATE,
referenced by enqueueForTrack()/regenerate()) to generate a unique jobId per
attempt (e.g., `waveform:${trackId}:${timestamp}` or append a UUID) so repeated
regenerate() calls actually enqueue new jobs, and ensure any code relying on the
old fixed jobId is adjusted accordingly; alternatively, before calling
waveformQueue.add remove/archive any existing failed job with jobId
`waveform:${trackId}` from BullMQ if you prefer keeping fixed IDs.
In `@backend/src/waveform.processor.ts`:
- Around line 42-43: The job currently sets PROCESSING via
waveformService.markProcessing and increments attempts but does not persist
intermediate failure info during retries; modify the catch paths in the
processor (the try/catch surrounding the waveform processing logic) to persist a
retryable state or at least the last error before rethrowing so the status API
doesn't show a forever-PROCESSING row. Concretely, add a call like
waveformService.recordAttemptError(trackId, err) or
waveformService.markRetrying(trackId, err, attemptCount) inside each catch
branch (the same place where you currently rethrow) so the DB is updated with
failReason/attempts for transient failures; keep the final
markFailed/markCompleted logic intact for terminal outcomes.
---
Nitpick comments:
In `@backend/src/mount-waveform/waveform-generator.service.ts`:
- Around line 5-9: The GeneratedWaveform.durationSeconds field is computed but
never persisted or returned; either remove it from GeneratedWaveform to avoid
confusion or persist and surface it: add a durationSeconds column to
WaveformEntity, update the markDone method in the waveform-generator service to
store generated.durationSeconds when saving peaks, and include durationSeconds
in WaveformStatusDto (and any response mapping) so clients receive the value.
- Around line 30-42: Replace the synchronous fs calls with their async
equivalents in the async method that contains this code: use await
fs.promises.access(filePath, fs.constants.R_OK) (or try/catch around await
fs.promises.stat(filePath)) to check existence and then await
fs.promises.stat(filePath) for stats; throw the same Error if access/stat fails.
Ensure peakCount calculation still uses stats.size and keep calls to
this.buildFakePeaks(peakCount, filePath) unchanged, and wrap the await calls in
try/catch to propagate the existing error behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0d6b82d-b015-427c-894c-8907f22ec29c
📒 Files selected for processing (12)
backend/src/mount-waveform/tracks.service.spec.tsbackend/src/mount-waveform/tracks.service.tsbackend/src/mount-waveform/waveform-generator.service.spec.tsbackend/src/mount-waveform/waveform-generator.service.tsbackend/src/mount-waveform/waveform.constants.tsbackend/src/mount-waveform/waveform.controller.spec.tsbackend/src/mount-waveform/waveform.controller.tsbackend/src/mount-waveform/waveform.dto.tsbackend/src/mount-waveform/waveform.entity.tsbackend/src/mount-waveform/waveform.module.tsbackend/src/mount-waveform/waveform.service.tsbackend/src/waveform.processor.ts
| import { WaveformService } from '../waveform/waveform.service'; | ||
| import { TrackEntity } from './track.entity'; | ||
| import { TracksService } from './tracks.service'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the actual location of track.entity.ts and waveform.service.ts
echo "=== Searching for track.entity.ts ==="
fd 'track.entity.ts'
echo "=== Searching for waveform.service.ts ==="
fd 'waveform.service.ts'
echo "=== Directory structure of mount-waveform ==="
tree backend/src/mount-waveform 2>/dev/null || find backend/src/mount-waveform -type f -name "*.ts" 2>/dev/nullRepository: OlufunbiIK/tip-tune
Length of output: 723
🏁 Script executed:
cat -n backend/src/mount-waveform/tracks.service.spec.ts | head -20Repository: OlufunbiIK/tip-tune
Length of output: 780
Fix import path for TrackEntity – module not found.
Line 5 imports TrackEntity from './track.entity', but the file does not exist in the mount-waveform directory. The actual location is backend/src/tracks/entities/track.entity.ts. Change the import to:
import { TrackEntity } from '../../tracks/entities/track.entity';
This is causing the pipeline failure TS2307: Cannot find module './track.entity'.
🧰 Tools
🪛 GitHub Actions: Backend CI
[error] 5-5: TS2307: Cannot find module './track.entity' or its corresponding type declarations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/mount-waveform/tracks.service.spec.ts` around lines 4 - 6, The
import for TrackEntity in tracks.service.spec.ts is pointing to a non-existent
'./track.entity'; update the import to reference the actual module location by
changing the TrackEntity import to the correct relative path (replace the
'./track.entity' import with the path leading to tracks/entities/track.entity),
so the TracksService tests can resolve TrackEntity correctly; ensure the import
line importing TrackEntity matches the module export name and compile.
| import { WaveformService } from '../waveform/waveform.service'; | ||
| import { CreateTrackDto } from './dto/create-track.dto'; | ||
| import { TrackEntity } from './track.entity'; |
There was a problem hiding this comment.
Import paths likely incorrect – same issues as spec file.
The import '../waveform/waveform.service' suggests a separate waveform/ directory, but other files in this PR place waveform code in mount-waveform/. Similarly, TrackEntity from './track.entity' caused a build failure in the spec file. Verify and correct these paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/mount-waveform/tracks.service.ts` around lines 9 - 11, The
imports for WaveformService, CreateTrackDto and TrackEntity are pointing to the
wrong module locations; update the import statements so WaveformService and any
waveform-related types are imported from the mount-waveform module where the
waveform code actually lives and ensure TrackEntity is imported from the module
that defines the entity (replace the incorrect '../waveform/waveform.service'
and './track.entity' imports with the correct module locations used elsewhere in
this PR so the spec/build can resolve WaveformService, CreateTrackDto and
TrackEntity).
| async createTrack(dto: CreateTrackDto): Promise<TrackEntity> { | ||
| const track = this.trackRepo.create(dto); | ||
| await this.trackRepo.save(track); | ||
|
|
||
| this.logger.log(`Track ${track.id} created – enqueuing waveform generation`); | ||
|
|
||
| // Durable – survives process restarts. No setTimeout. | ||
| await this.waveformService.enqueueForTrack(track.id, track.audioFilePath); | ||
|
|
||
| return track; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify TrackEntity has audioFilePath field
echo "=== Searching for TrackEntity definition ==="
ast-grep --pattern $'class TrackEntity {
$$$
}'
echo "=== Searching for audioFilePath in track entities ==="
rg -n 'audioFilePath' --type tsRepository: OlufunbiIK/tip-tune
Length of output: 2035
🏁 Script executed:
#!/bin/bash
# Find track entity and DTO files
echo "=== Finding track-related entity/DTO files ==="
fd -e ts -i track | grep -E "(entity|dto)" | head -20
echo "=== Searching for CreateTrackDto definition ==="
rg -n 'class CreateTrackDto|interface CreateTrackDto' -A 10 --type ts
echo "=== Searching for TrackEntity definition ==="
rg -n 'class TrackEntity|interface TrackEntity|export class TrackEntity' -A 15 --type tsRepository: OlufunbiIK/tip-tune
Length of output: 1474
🏁 Script executed:
#!/bin/bash
# Read the actual entity and DTO files
echo "=== TrackEntity (backend/src/tracks/entities/track.entity.ts) ==="
cat backend/src/tracks/entities/track.entity.ts
echo ""
echo "=== CreateTrackDto (backend/src/tracks/dto/create-track.dto.ts) ==="
cat backend/src/tracks/dto/create-track.dto.tsRepository: OlufunbiIK/tip-tune
Length of output: 4861
Fix field mismatch: audioFilePath does not exist on Track entity.
The Track entity defines audioUrl, not audioFilePath. Accessing track.audioFilePath on line 40 returns undefined, which is then passed to enqueueForTrack(), causing waveform processing to fail silently. Change track.audioFilePath to track.audioUrl.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/mount-waveform/tracks.service.ts` around lines 33 - 43, In
createTrack (tracks.service.ts) the code passes a non-existent field
track.audioFilePath to waveformService.enqueueForTrack causing undefined to be
enqueued; change the argument to use the Track entity's actual field
track.audioUrl (i.e., call enqueueForTrack(track.id, track.audioUrl)), keeping
the rest of the createTrack logic and logger the same so
waveformService.enqueueForTrack receives the correct URL value.
| @@ -0,0 +1,81 @@ | |||
| import { NotFoundException } from '@nestjs/common'; | |||
| import { Test, TestingModule } from '@nestjs/testing'; | |||
| import { WaveformStatus } from './dto/waveform.dto'; | |||
There was a problem hiding this comment.
This spec cannot compile until the DTO module is wired up.
Backend CI is already failing with TS2307 on Line 3, so ./dto/waveform.dto is either missing from the PR or the relative path is wrong. Please add the DTO file or fix the import before merge.
🧰 Tools
🪛 GitHub Actions: Backend CI
[error] 3-3: TS2307: Cannot find module './dto/waveform.dto' or its corresponding type declarations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/mount-waveform/waveform.controller.spec.ts` at line 3, The test
import for WaveformStatus in waveform.controller.spec.ts fails because the DTO
module is missing or the relative path is incorrect; either add the missing DTO
file exporting WaveformStatus (e.g., export enum WaveformStatus { ... } or
export type/const as used) or correct the import path to the actual location of
the DTO, then ensure the exported symbol name matches the import
(WaveformStatus) so TypeScript can resolve './dto/waveform.dto' in the test.
| @HttpCode(HttpStatus.OK) | ||
| @ApiOperation({ | ||
| summary: 'Force-regenerate the waveform for a track', | ||
| description: | ||
| 'Enqueues a durable BullMQ job. Returns 200 + already_processing if ' + | ||
| 'a job is already in flight.', | ||
| }) | ||
| @ApiOkResponse({ type: RegenerateResponseDto }) | ||
| @ApiNotFoundResponse({ description: 'No waveform record exists for track' }) | ||
| @ApiConflictResponse({ description: 'Waveform generation already in progress' }) |
There was a problem hiding this comment.
Swagger advertises a 409 that this handler never emits.
Line 47 fixes the response code at 200, and the busy case is returned as { result: 'already_processing' } rather than a thrown conflict. Either switch that path to ConflictException or drop @ApiConflictResponse so the published contract matches runtime behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/mount-waveform/waveform.controller.ts` around lines 47 - 56, The
Swagger decorator `@ApiConflictResponse` documents a 409 but the handler always
returns 200 with { result: 'already_processing' }; update the runtime to match
the contract or update the docs: either (A) change the busy branch in the
controller method (the method that returns RegenerateResponseDto / currently
returns { result: 'already_processing' }) to throw new
ConflictException('Waveform generation already in progress') so the endpoint
actually emits a 409, or (B) remove the `@ApiConflictResponse` decorator and any
mention of a 409 so the OpenAPI spec matches the current behavior; locate
references to HttpCode(HttpStatus.OK), RegenerateResponseDto and the handler
method in waveform.controller.ts to make the change.
| import { WaveformController } from './waveform.controller'; | ||
| import { WAVEFORM_QUEUE } from './waveform.constants'; | ||
| import { WaveformEntity } from './waveform.entity'; | ||
| import { WaveformProcessor } from './waveform.processor'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the actual location of WaveformProcessor
echo "=== Searching for waveform.processor.ts ==="
fd 'waveform.processor.ts'Repository: OlufunbiIK/tip-tune
Length of output: 140
🏁 Script executed:
#!/bin/bash
# Check if file exists at the path the import expects
echo "=== Checking for file at mount-waveform level ==="
ls -la backend/src/mount-waveform/waveform.processor.ts 2>&1
echo -e "\n=== Listing mount-waveform directory ==="
ls -la backend/src/mount-waveform/
echo -e "\n=== Verifying processor file location ==="
cat backend/src/waveform.processor.ts | head -20Repository: OlufunbiIK/tip-tune
Length of output: 2056
Fix incorrect import path for WaveformProcessor.
The import references './waveform.processor' but the file is located at backend/src/waveform.processor.ts (outside the mount-waveform directory). Change the import to '../waveform.processor'.
Additionally, note that waveform.processor.ts itself has broken imports (lines importing from './waveform-generator.service', './waveform.service', './waveform.constants') that reference files in mount-waveform/ but will fail to resolve from the root level.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/mount-waveform/waveform.module.ts` at line 9, The import of
WaveformProcessor in waveform.module.ts is wrong; change the module's import for
the symbol WaveformProcessor to reference the parent-module location (use the
parent-relative path so it resolves to the file that exports WaveformProcessor).
Also open waveform.processor.ts and correct its broken imports: update the
imports for WaveformGeneratorService, WaveformService, and any waveform
constants so they point to the actual locations of those symbols (adjust the
relative paths so the imports resolve from the processor file's location).
Ensure all three affected symbols (WaveformProcessor, WaveformGeneratorService,
WaveformService, and waveform constants) import using correct relative paths so
the TypeScript compiler can resolve them.
| if ( | ||
| record && | ||
| [WaveformStatus.PENDING, WaveformStatus.PROCESSING].includes( | ||
| record.status, | ||
| ) | ||
| ) { | ||
| this.logger.log( | ||
| `Waveform already ${record.status} for track ${trackId} – skipping enqueue`, | ||
| ); | ||
| return record.bullJobId ?? ''; | ||
| } |
There was a problem hiding this comment.
A crash between the DB save and queue.add() can strand tracks permanently.
Lines 79-96 persist PENDING before the BullMQ job exists and only backfill bullJobId afterward. If that window fails, Lines 56-65 will treat the orphaned row as in-flight forever and enqueueForTrack() just returns ''. Re-check bullJobId / queue liveness before short-circuiting, or use a safer DB-to-queue handoff.
Also applies to: 79-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/mount-waveform/waveform.service.ts` around lines 56 - 66, The
short-circuit in enqueueForTrack that returns when record.status is
PENDING/PROCESSING can permanently orphan tracks because the DB row is written
before the BullMQ job is created; update enqueueForTrack to verify the presence
and liveness of record.bullJobId (and/or the corresponding job in the queue)
before returning, or invert the handoff so you create the BullMQ job first and
then persist record.bullJobId in the DB (or perform both operations inside a
transaction/atomic upsert). Concretely, change the logic around the check of
record.status / record.bullJobId so that if status is PENDING/PROCESSING you
still confirm record.bullJobId is non-empty and the job exists in the queue (or
re-enqueue and update DB) rather than immediately returning '', and when
creating a new job call queue.add() first (or use a DB transaction) and persist
the returned job id into record.bullJobId to avoid the race.
| const job = await this.waveformQueue.add( | ||
| WAVEFORM_JOBS.GENERATE, | ||
| { trackId, audioFilePath }, | ||
| { | ||
| jobId: `waveform:${trackId}`, | ||
| attempts: WAVEFORM_JOB_DEFAULTS.ATTEMPTS, | ||
| backoff: { | ||
| type: 'exponential', | ||
| delay: WAVEFORM_JOB_DEFAULTS.BACKOFF_DELAY_MS, | ||
| }, | ||
| removeOnComplete: true, | ||
| removeOnFail: false, // keep failed jobs in BullMQ dashboard | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "waveform.service.ts" -o -name "*waveform*" -type f | head -20Repository: OlufunbiIK/tip-tune
Length of output: 903
🏁 Script executed:
cat -n backend/src/mount-waveform/waveform.service.ts | head -120Repository: OlufunbiIK/tip-tune
Length of output: 4071
🏁 Script executed:
cat -n backend/src/mount-waveform/waveform.service.ts | tail -50Repository: OlufunbiIK/tip-tune
Length of output: 1880
🏁 Script executed:
cat -n backend/src/mount-waveform/waveform.service.ts | sed -n '110,154p'Repository: OlufunbiIK/tip-tune
Length of output: 1832
🏁 Script executed:
rg -A 30 "regenerate" backend/src/mount-waveform/waveform.service.tsRepository: OlufunbiIK/tip-tune
Length of output: 1292
🏁 Script executed:
cat -n backend/src/mount-waveform/waveform.constants.tsRepository: OlufunbiIK/tip-tune
Length of output: 467
🏁 Script executed:
rg -B 5 -A 5 "queue.add" backend/src/mount-waveform/waveform.service.tsRepository: OlufunbiIK/tip-tune
Length of output: 45
🌐 Web query:
BullMQ jobId deduplication existing job same id behavior
💡 Result:
In BullMQ, when adding a job with a custom jobId that already exists in the same queue (in any state: waiting, active, delayed, completed, or failed), the new job is ignored and not added to the queue. This enforces uniqueness of jobIds per queue and prevents duplicates. The operation succeeds without throwing an error, but a 'duplicated' event is emitted on QueueEvents, providing details like the existing jobId. This behavior only applies if the existing job has not been removed (e.g., via removeOnComplete: true, removeOnFail: true, manual removal, or queue cleaning). Once removed, the jobId becomes available again. Deduplication is a separate, optional feature using { deduplication: { id: 'customValue' } } for more advanced control (e.g., TTL-based), but basic jobId uniqueness provides simple deduplication out-of-the-box. Example: await queue.add('job', data, { jobId: 'my-id' }); // Adds job await queue.add('job', data2, { jobId: 'my-id' }); // Ignored, emits 'duplicated' Note: Custom jobIds must avoid ':' character due to Redis key conventions.
Citations:
- 1: https://docs.bullmq.io/guide/jobs/job-ids
- 2: https://docs.bullmq.io/guide/jobs/deduplication
- 3: [Question]: Return value of Queue.add when there is a duplicate with a custom job ID taskforcesh/bullmq#3214
- 4: https://docs.bullmq.net/http-api/queues/adding-jobs/custom-job-ids
- 5: https://docs.bullmq.io/patterns/throttle-jobs
🏁 Script executed:
rg -B 10 -A 10 "queue.add\|waveformQueue.add" backend/src/mount-waveform/Repository: OlufunbiIK/tip-tune
Length of output: 45
🏁 Script executed:
rg "duplicated\|ConflictException" backend/src/mount-waveform/Repository: OlufunbiIK/tip-tune
Length of output: 45
regenerate() silently fails when a job with the same id exists from a previous failure.
BullMQ silently ignores calls to queue.add() when a job with the given jobId already exists in the queue, without throwing an error. With removeOnFail: false on line 92, a failed job persists with jobId waveform:${trackId}. When regenerate() is called on the same track, it invokes enqueueForTrack(), which skips the early return only if the DB status is not PENDING or PROCESSING (i.e., when status is FAILED). The subsequent queue.add() call with the same jobId silently fails to enqueue, but regenerate() still returns a success response to the caller. Use a unique jobId per generation attempt (e.g., with a timestamp or UUID), or remove/archive the failed job from BullMQ before attempting to regenerate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/mount-waveform/waveform.service.ts` around lines 81 - 93, The
enqueue step silently fails when a previous failed job with jobId
`waveform:${trackId}` still exists (removeOnFail: false); update the enqueue
logic (where waveformQueue.add is called for WAVEFORM_JOBS.GENERATE, referenced
by enqueueForTrack()/regenerate()) to generate a unique jobId per attempt (e.g.,
`waveform:${trackId}:${timestamp}` or append a UUID) so repeated regenerate()
calls actually enqueue new jobs, and ensure any code relying on the old fixed
jobId is adjusted accordingly; alternatively, before calling waveformQueue.add
remove/archive any existing failed job with jobId `waveform:${trackId}` from
BullMQ if you prefer keeping fixed IDs.
| // Derive audio path stored during original enqueue – in a real app you | ||
| // would look it up from the tracks table. Here we re-use the job payload | ||
| // that BullMQ might still hold, or fall back to a placeholder so the | ||
| // worker can re-resolve it. | ||
| const existingJob = record.bullJobId | ||
| ? await this.waveformQueue.getJob(record.bullJobId) | ||
| : null; | ||
|
|
||
| const audioFilePath = | ||
| (existingJob?.data as WaveformJobPayload | null)?.audioFilePath ?? | ||
| `__RESOLVE_FROM_TRACK__${trackId}`; | ||
|
|
||
| const jobId = await this.enqueueForTrack(trackId, audioFilePath); | ||
| return { result: 'queued', jobId }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find backend/src -type f -name "*.ts" | grep -E "(waveform|mount-waveform)"Repository: OlufunbiIK/tip-tune
Length of output: 964
🏁 Script executed:
wc -l backend/src/mount-waveform/waveform.service.tsRepository: OlufunbiIK/tip-tune
Length of output: 113
🏁 Script executed:
wc -l backend/src/mount-waveform/waveform-generator.service.tsRepository: OlufunbiIK/tip-tune
Length of output: 122
🏁 Script executed:
cat -n backend/src/mount-waveform/waveform.service.ts | sed -n '85,145p'Repository: OlufunbiIK/tip-tune
Length of output: 2328
🏁 Script executed:
cat -n backend/src/mount-waveform/waveform-generator.service.ts | sed -n '25,40p'Repository: OlufunbiIK/tip-tune
Length of output: 952
🏁 Script executed:
cat -n backend/src/waveform.processor.ts | head -100Repository: OlufunbiIK/tip-tune
Length of output: 2907
🏁 Script executed:
rg "__RESOLVE_FROM_TRACK__" -B 5 -A 5Repository: OlufunbiIK/tip-tune
Length of output: 909
🏁 Script executed:
cat -n backend/src/mount-waveform/waveform.service.ts | sed -n '60,85p'Repository: OlufunbiIK/tip-tune
Length of output: 873
regenerate() fails with "Audio file not found" after successful waveform generation.
After a job completes successfully, removeOnComplete: true (line 91) deletes it from BullMQ. When regenerate() is called again, the job is gone, so it falls back to the placeholder __RESOLVE_FROM_TRACK__${trackId} (line 137). This placeholder is never resolved—it's passed directly through the processor to the generator's generateFromFile(), which immediately throws "Audio file not found" at line 30-31 of waveform-generator.service.ts because the placeholder is not a real file path.
Store the real audioFilePath in the database (e.g., in the waveform record), or resolve it from the tracks table before queueing the regeneration job.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/mount-waveform/waveform.service.ts` around lines 127 - 140,
regenerate() falls back to the placeholder __RESOLVE_FROM_TRACK__${trackId} when
the BullMQ job was removed (removeOnComplete: true), which results in
waveform-generator.generateFromFile() getting an invalid path and throwing;
update the logic so the real audioFilePath is available when queuing a
regeneration: either persist audioFilePath to the waveform record when you first
enqueue (save the value used in enqueueForTrack) and read it here (replace the
placeholder), or, if you prefer not to store it, resolve the path from the
tracks table before calling enqueueForTrack; change the code paths involving
record.bullJobId, waveformQueue.getJob, WaveformJobPayload, audioFilePath and
enqueueForTrack to prefer the persisted/resolved audioFilePath over the
__RESOLVE_FROM_TRACK__ placeholder.
| await this.waveformService.markProcessing(trackId); | ||
| await this.waveformService.incrementAttemptCount(trackId); |
There was a problem hiding this comment.
Retry backoff is invisible to the status API.
After Lines 42-43 mark the row PROCESSING, the catch block only persists anything on the last attempt. During BullMQ backoff, clients will still see PROCESSING with no failReason even though the worker is idle and waiting to retry. Persist a retryable state, or at least the last error, before rethrowing.
Also applies to: 50-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/waveform.processor.ts` around lines 42 - 43, The job currently
sets PROCESSING via waveformService.markProcessing and increments attempts but
does not persist intermediate failure info during retries; modify the catch
paths in the processor (the try/catch surrounding the waveform processing logic)
to persist a retryable state or at least the last error before rethrowing so the
status API doesn't show a forever-PROCESSING row. Concretely, add a call like
waveformService.recordAttemptError(trackId, err) or
waveformService.markRetrying(trackId, err, attemptCount) inside each catch
branch (the same place where you currently rethrow) so the DB is updated with
failReason/attempts for transient failures; keep the final
markFailed/markCompleted logic intact for terminal outcomes.
2 participants
Closes #346
Summary by CodeRabbit
Release Notes
New Features
Tests