Skip to content

fix(core): reject findCreateFind when recovery fails - #18298

Open
orlandohohmeier wants to merge 1 commit into
sequelize:mainfrom
orlandohohmeier:oh/fix-findcreatefind-recovery
Open

fix(core): reject findCreateFind when recovery fails#18298
orlandohohmeier wants to merge 1 commit into
sequelize:mainfrom
orlandohohmeier:oh/fix-findcreatefind-recovery

Conversation

@orlandohohmeier

@orlandohohmeier orlandohohmeier commented Aug 4, 2026

Copy link
Copy Markdown

findCreateFind promises an instance when it resolves. Rethrow the create error if the duplicate-recovery lookup finds no row, matching findOrCreate instead of resolving with null.

Pull Request Checklist

  • Have you added new tests to prevent regressions?
  • If a documentation update is necessary, have you opened a PR to the documentation repository?
  • Did you update the typescript typings accordingly (if applicable)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Does the name of your PR follow our conventions?

Description of Changes

List of Breaking Changes

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling when a create operation fails and the recovery lookup finds no matching record.
    • The original error is now preserved and reported instead of returning an empty result.
  • Tests

    • Added coverage for recovery scenarios involving empty-result and uniqueness errors.
    • Verified that the original error is propagated correctly.

@orlandohohmeier
orlandohohmeier requested a review from a team as a code owner August 4, 2026 16:11
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 78f34c5e-4d94-4043-b1ad-c30719531fda

📥 Commits

Reviewing files that changed from the base of the PR and between a4c4145 and 8781e50.

📒 Files selected for processing (2)
  • packages/core/src/model.js
  • packages/core/test/unit/model/find-create-find.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/core/src/model.js
  • packages/core/test/unit/model/find-create-find.test.js

📝 Walkthrough

Walkthrough

findCreateFind now rethrows the original creation error when the recovery lookup returns no record. Tests cover empty-result and unique-constraint errors.

Changes

findCreateFind error propagation

Layer / File(s) Summary
Rethrow and regression coverage
packages/core/src/model.js, packages/core/test/unit/model/find-create-find.test.js
findCreateFind rethrows the original error when the recovery findOne call returns null. Parameterized tests verify error identity and two recovery lookups for EmptyResultError and UniqueConstraintError.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: sdepold

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the change to reject findCreateFind when recovery fails.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Biome (2.5.6)
packages/core/src/model.js

File contains syntax errors that prevent linting: Line 3: Illegal use of an import declaration outside of a module; Line 4: Illegal use of an import declaration outside of a module; Line 5: Illegal use of an import declaration outside of a module; Line 6: Illegal use of an import declaration outside of a module; Line 7: Illegal use of an import declaration outside of a module; Line 8: Illegal use of an import declaration outside of a module; Line 9: Illegal use of an import declaration outside of a module; Line 10: Illegal use of an import declaration outside of a module; Line 11: Illegal use of an import declaration outside of a module; Line 12: Illegal use of an import declaration outside of a module; Line 13: Illegal use of an import declaration outside of a module; Line 14: Illegal use of an import declaration outside of a module; Line 15: Illegal use of an import declaration outside of a module; Line 16: Illegal use of an import declaration outside of a module; Line 17: Illegal us

... [truncated 1592 characters] ...

e 53: Illegal use of an import declaration outside of a module; Line 54: Illegal use of an import declaration outside of a module; Line 61: Illegal use of an import declaration outside of a module; Line 62: Illegal use of an import declaration outside of a module; Line 63: Illegal use of an import declaration outside of a module; Line 64: Illegal use of an import declaration outside of a module; Line 65: Illegal use of an import declaration outside of a module; Line 72: Illegal use of an import declaration outside of a module; Line 73: Illegal use of an import declaration outside of a module; Line 74: Illegal use of an import declaration outside of a module; Line 75: Illegal use of an import declaration outside of a module; Line 148: Illegal use of an export declaration outside of a module

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment on lines +2122 to +2124
if (foundAgain === null) {
throw error;
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still kinda back and forth on whether to re-throw here, mirroring findOrCreate and in line with what I would expect from a method returning a Promise, or adjust the type to make the potential null case explicit.

findCreateFind promises an instance when it resolves. Rethrow the create error if the duplicate-recovery lookup finds no row, matching findOrCreate instead of resolving with null.

Co-Authored-By: Codex <noreply@openai.com>
@orlandohohmeier
orlandohohmeier force-pushed the oh/fix-findcreatefind-recovery branch from a4c4145 to 8781e50 Compare August 4, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant