starred/audiobookshelf
1
0
Fork 0
mirror of https://github.com/advplyr/audiobookshelf.git synced 2026-05-30 23:40:40 +02:00
Code Issues 1.0k Packages Projects Releases 10 Wiki Activity

[PR #5118] fix: Handle duplicate series name error on rename instead of crashing #4431

New Issue
Open
opened 2026-04-25 00:19:43 +02:00 by adam · 0 comments
adam commented 2026-04-25 00:19:43 +02:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/advplyr/audiobookshelf/pull/5118
Author: @Xalior
Created: 3/11/2026
Status: 🔄 Open

Base: master ← Head: fix/series-rename-unique-constraint-crash

📝 Commits (1)

  • b58ddaf fix: Handle duplicate series name error on rename instead of crashing

📊 Changes

2 files changed (+194 additions, -1 deletions)

View changed files

📝 server/controllers/SeriesController.js (+9 -1)
➕ test/server/controllers/SeriesController.test.js (+185 -0)

📄 Description

Brief summary

PATCH /api/series/:id with a duplicate name crashes the server instead of returning an error.

Which issue is fixed?

Related to #3846. Complementary to #4962 (which adds pre-validation in the UI but still lacks server-side error handling for race conditions).

In-depth Description

The update() method in SeriesController.js calls await req.series.save() with no try/catch. When renaming a series to a name that already exists in the same library, the unique_series_name_per_library constraint (added in PR #3417 / v2.15.0) throws a SequelizeUniqueConstraintError. This propagates as an unhandled promise rejection, which crashes the server process with exit code 1.

The fix wraps save() in a try/catch and returns HTTP 400 with a descriptive message for unique constraint violations. All other errors are re-thrown to preserve existing behavior.

How have you tested this?

  • Added test/server/controllers/SeriesController.test.js with 6 tests (mocha/chai/sinon, in-memory SQLite)
  • The test "should return 400 when renaming to a name that already exists in the same library" also serves as a minimal reproducer: run it against the unpatched SeriesController and it throws the unhandled SequelizeUniqueConstraintError — the same error seen in production as a fatal crash at SeriesController.js:83
  • Full test suite passes (341 tests, 0 failures)

Unpatched (bug reproduced):

1) should return 400 when renaming to a name that already exists in the same library:
   Validation error
   Error at Database.<anonymous> (sequelize/lib/dialects/sqlite/query.js:185:27)
   ...
   at async SeriesController.update (server/controllers/SeriesController.js:83:7)

Patched (bug fixed):

  6 passing (575ms)

Screenshots

N/A — API-only change

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/advplyr/audiobookshelf/pull/5118 **Author:** [@Xalior](https://github.com/Xalior) **Created:** 3/11/2026 **Status:** 🔄 Open **Base:** `master` ← **Head:** `fix/series-rename-unique-constraint-crash` --- ### 📝 Commits (1) - [`b58ddaf`](https://github.com/advplyr/audiobookshelf/commit/b58ddaf9d3bd13c1d9544af4eb3058e33c3ab6f2) fix: Handle duplicate series name error on rename instead of crashing ### 📊 Changes **2 files changed** (+194 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `server/controllers/SeriesController.js` (+9 -1) ➕ `test/server/controllers/SeriesController.test.js` (+185 -0) </details> ### 📄 Description ## Brief summary `PATCH /api/series/:id` with a duplicate name crashes the server instead of returning an error. ## Which issue is fixed? Related to #3846. Complementary to #4962 (which adds pre-validation in the UI but still lacks server-side error handling for race conditions). ## In-depth Description The `update()` method in `SeriesController.js` calls `await req.series.save()` with no try/catch. When renaming a series to a name that already exists in the same library, the `unique_series_name_per_library` constraint (added in PR #3417 / v2.15.0) throws a `SequelizeUniqueConstraintError`. This propagates as an unhandled promise rejection, which crashes the server process with exit code 1. The fix wraps `save()` in a try/catch and returns HTTP 400 with a descriptive message for unique constraint violations. All other errors are re-thrown to preserve existing behavior. ## How have you tested this? - Added `test/server/controllers/SeriesController.test.js` with 6 tests (mocha/chai/sinon, in-memory SQLite) - The test `"should return 400 when renaming to a name that already exists in the same library"` also serves as a minimal reproducer: run it against the **unpatched** `SeriesController` and it throws the unhandled `SequelizeUniqueConstraintError` — the same error seen in production as a fatal crash at `SeriesController.js:83` - Full test suite passes (341 tests, 0 failures) **Unpatched (bug reproduced):** ``` 1) should return 400 when renaming to a name that already exists in the same library: Validation error Error at Database.<anonymous> (sequelize/lib/dialects/sqlite/query.js:185:27) ... at async SeriesController.update (server/controllers/SeriesController.js:83:7) ``` **Patched (bug fixed):** ``` 6 passing (575ms) ``` ## Screenshots N/A — API-only change --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2026-04-25 00:19:43 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
authentication
backlog
bug
chapter editor
config-issue
ebooks
encoding/embedding
enhancement
help wanted
listening sessions & progress
planned
possible plugin
progress sync
pull-request
Mirrored from GitHub Pull Request
 sorting/filtering/searching
unable to reproduce
upload
users & permissions
waiting
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam (Adam Melkus)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/audiobookshelf#4431
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?