fix: Allow Parse Server startup without optional push adapter#10446
fix: Allow Parse Server startup without optional push adapter#10446ga262 wants to merge 3 commits intoparse-community:alphafrom
Conversation
Signed-off-by: ga262 <37159881+ga262@users.noreply.github.com>
Added a function to check for missing push adapter module and handle errors accordingly. Signed-off-by: ga262 <37159881+ga262@users.noreply.github.com>
Removed '@parse/push-adapter' from dependencies and added it to optionalDependencies. Signed-off-by: ga262 <37159881+ga262@users.noreply.github.com>
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
Bot
changed the title
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
📝 WalkthroughWalkthroughThis change converts Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (5 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Controllers/index.js (1)
172-201:⚠️ Potential issue | 🟠 MajorOnly load the default push adapter when it is actually needed.
Line 191 unconditionally imports the optional default adapter even when
pushis unset or a custompush.adapteris provided. This can cause startup to fail unnecessarily when the adapter is not used. Additionally, the error detection at line 176 usingmessage.includes('@parse/push-adapter')can misclassify transitive dependency failures from within@parse/push-adapteras a missing package if the error message path happens to contain that string.Move the import into the conditional block that only executes when actually needed:
Suggested fix
function isPushAdapterModuleMissing(error: any): boolean { const message = `${error?.message || error || ''}`; const hasMissingCode = error?.code === 'ERR_MODULE_NOT_FOUND' || error?.code === 'MODULE_NOT_FOUND'; - return hasMissingCode && message.includes('@parse/push-adapter'); + return ( + hasMissingCode && + /Cannot find (?:package|module) ['"]@parse\/push-adapter['"]/.test(message) + ); } // Pass the push options too as it works with the default let ParsePushAdapter; - try { - ParsePushAdapter = await loadModule('@parse/push-adapter'); - } catch (error) { - if (!isPushAdapterModuleMissing(error)) { - throw error; - } - if (push && !pushOptions.adapter) { + if (push && !pushOptions.adapter) { + try { + ParsePushAdapter = await loadModule('@parse/push-adapter'); + } catch (error) { + if (!isPushAdapterModuleMissing(error)) { + throw error; + } throw new Error( 'Push is configured but the optional dependency "@parse/push-adapter" is not installed. Install "@parse/push-adapter" or configure "push.adapter".' ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Controllers/index.js` around lines 172 - 201, The code currently calls loadModule('@parse/push-adapter') unconditionally and uses isPushAdapterModuleMissing(error) which naively checks message.includes('@parse/push-adapter'), causing unnecessary startup failures and false positives; update getPushController so ParsePushAdapter is only loaded when push is truthy and pushOptions.adapter is not provided (i.e. move the await loadModule('@parse/push-adapter') call inside the conditional that checks if push && !pushOptions.adapter), and harden isPushAdapterModuleMissing(error) to detect a missing package more precisely by checking error.code for 'ERR_MODULE_NOT_FOUND' or 'MODULE_NOT_FOUND' and matching the error message against a stricter pattern that looks for "Cannot find module" (or the module name quoted) specifically for '@parse/push-adapter' rather than a simple substring match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/Controllers/index.js`:
- Around line 172-201: The code currently calls
loadModule('@parse/push-adapter') unconditionally and uses
isPushAdapterModuleMissing(error) which naively checks
message.includes('@parse/push-adapter'), causing unnecessary startup failures
and false positives; update getPushController so ParsePushAdapter is only loaded
when push is truthy and pushOptions.adapter is not provided (i.e. move the await
loadModule('@parse/push-adapter') call inside the conditional that checks if
push && !pushOptions.adapter), and harden isPushAdapterModuleMissing(error) to
detect a missing package more precisely by checking error.code for
'ERR_MODULE_NOT_FOUND' or 'MODULE_NOT_FOUND' and matching the error message
against a stricter pattern that looks for "Cannot find module" (or the module
name quoted) specifically for '@parse/push-adapter' rather than a simple
substring match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 950bd482-0f92-4d9f-b1b7-633395bf13d3
📒 Files selected for processing (3)
package.jsonspec/index.spec.jssrc/Controllers/index.js
1 participant
Issue
Parse Server currently declares
@parse/push-adapteras an optional dependency, but startup still attempts to load it unconditionally in the push controller path. This creates a mismatch between package metadata and runtime behavior.Current behavior
@parse/push-adapteris not installed:push.adapter, startup fails with a generic module resolution errorExpected behavior
pushis not configured, Parse Server should start even if@parse/push-adapteris absentpushis configured but no custom adapter is provided, Parse Server should fail with a clear actionable error indicating that@parse/push-adaptermust be installed (orpush.adaptermust be provided)Why this matters
optionalDependenciesintent--omit=optional)Summary
@parse/push-adapterwhen loading the default push adapterpushis not configured and the optional adapter is not installedpushis configured but neither@parse/push-adapternor a custompush.adapteris availablePushWorkerwhen push support is enabledTest plan
npm run buildnpm run testonly spec/index.spec.jsnpm run testonly spec/AdapterLoader.spec.jspushisundefinedand@parse/push-adapteris missingpushis configured and@parse/push-adapteris missingSummary by CodeRabbit
Release Notes
New Features
Improvements