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

Server: Fixes #10118: Missing record validation before trying to add item to user #10471

Merged
merged 3 commits into from
Jun 4, 2024
Merged

Server: Fixes #10118: Missing record validation before trying to add item to user #10471

merged 3 commits into from
Jun 4, 2024

Conversation

pedr
Copy link
Collaborator

@pedr pedr commented May 24, 2024

Fixes #10118

The error:

app-1        | 07:50:18 0|app  | 2024-03-14 07:50:17: [error] TaskService: On #10 (Process orphaned items) TypeError: Cannot use 'in' operator to search for 'name' in undefined
app-1        | 07:50:18 0|app  |     at UserItemModel.<anonymous> (/home/joplin/packages/server/src/models/UserItemModel.ts:126:17)
app-1        | 07:50:18 0|app  |     at Generator.next (<anonymous>)
app-1        | 07:50:18 0|app  |     at /home/joplin/packages/server/dist/models/UserItemModel.js:8:71
app-1        | 07:50:18 0|app  |     at new Promise (<anonymous>)
app-1        | 07:50:18 0|app  |     at __awaiter (/home/joplin/packages/server/dist/models/UserItemModel.js:4:12)
app-1        | 07:50:18 0|app  |     at /home/joplin/packages/server/src/models/UserItemModel.ts:124:41
app-1        | 07:50:18 0|app  |     at UserItemModel.<anonymous> (/home/joplin/packages/server/src/models/BaseModel.ts:221:19)
app-1        | 07:50:18 0|app  |     at Generator.next (<anonymous>)
app-1        | 07:50:18 0|app  |     at fulfilled (/home/joplin/packages/server/dist/models/BaseModel.js:5:58)
app-1        | 07:50:18 0|app  |     at processTicksAndRejections (node:internal/process/task_queues:95:5)

While I could not reproduce the error, I think it is safe to assume that it could happen during the ItemModel.processOrphanedItems.

Basically what would happen is that a item would be orphaned, the processOrphanedItems would start and while it was happening the file was deleted before the process had time added the item back to the user collection.

That means that when UserItemModel.add happened, it would not found a item record anymore, which end up causing the error inside the UserItemModel.addMulti.

Testing

At first, I tried to add a more complex test that emulated the behavior, but it was getting very complicated and taking a lot of time, If the reviewer's opinion is different I can try to reproduce the behavior, but the simple test I added already will cover the potential regression if someone removes one of the checks.

@pedr pedr added bug It's a bug server Issues related to Joplin Server labels May 24, 2024
@pedr pedr requested a review from laurent22 May 24, 2024 22:42
@pedr
Copy link
Collaborator Author

pedr commented May 27, 2024

  • Add a error message on add( function and remove validation on addMulti

@laurent22 laurent22 merged commit 19f0b66 into laurent22:dev Jun 4, 2024
10 checks passed
@pedr pedr deleted the add-undefined-check-user-item-add branch June 5, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It's a bug server Issues related to Joplin Server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError when running "process orphaned items" on Joplin server
2 participants