feat: Add Parse Server option verifyServerUrl to disable server URL verification on server launch
#9881
Conversation
|
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! ❌ Please fill out all fields with a placeholder |
|
Warning Rate limit exceeded@mtrezza has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 13 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new boolean option Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant PS as ParseServer.startApp
participant API as express app / mount handler
participant Verifier as ParseServer.verifyServerUrl
Caller ->> PS: startApp(options)
PS ->> API: configure listeners / mount callbacks
alt options.verifyServerUrl === false
API --x Verifier: verification skipped (not awaited)
Note right of API #DDF2E9: startup proceeds without self-call
else
API ->> Verifier: await verifyServerUrl()
Verifier -->> API: verification result
end
API ->> PS: continue startup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Options/Definitions.js (1)
596-602: Consider adding a security warning to the documentation.The option definition is technically correct and follows the established pattern. However, given the security implications of bypassing server URL verification, consider strengthening the documentation with an explicit warning.
Apply this diff to add a security note:
skipVerifyServerUrl: { env: 'PARSE_SERVER_SKIP_VERIFY_SERVER_URL', help: - 'Set to `true` to skip the server URL verification on startup. This can be useful in environments where the server URL is not accessible from the server itself, such as when running behind a firewall, in certain containerized environments, or in test environments like Jest where the verification may cause issues or unnecessary delays during test execution.<br><br>Default is `false`.', + 'Set to `true` to skip the server URL verification on startup. This can be useful in environments where the server URL is not accessible from the server itself, such as when running behind a firewall, in certain containerized environments, or in test environments like Jest where the verification may cause issues or unnecessary delays during test execution.<br><br>⚠️ Only enable this option when necessary, as server URL verification helps detect configuration errors early. Skipping verification may hide issues that could affect Cloud Code and push notifications.<br><br>Default is `false`.', action: parsers.booleanParser, default: false, },src/ParseServer.ts (1)
377-382: Consider adding a log message when verification is skipped.The conditional logic correctly skips verification when
skipVerifyServerUrlistrue. However, adding an informational log message would help with debugging and make it clear to operators that verification was intentionally skipped.Apply this diff to add logging:
// verify the server url after a 'mount' event is received /* istanbul ignore next */ if (!skipVerifyServerUrl) { api.on('mount', async function () { await new Promise(resolve => setTimeout(resolve, 1000)); ParseServer.verifyServerUrl(); }); + } else { + const logger = (logging as any).logger; + logger.info('Server URL verification is disabled via skipVerifyServerUrl option'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Options/Definitions.js(1 hunks)src/Options/docs.js(1 hunks)src/Options/index.js(1 hunks)src/ParseServer.ts(2 hunks)types/Options/index.d.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Options/Definitions.js (1)
resources/buildConfigDefinitions.js (1)
parsers(12-12)
🪛 Biome (2.1.2)
src/Options/index.js
[error] 64-64: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: Node 20
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Node 18
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Redis Cache
- GitHub Check: Docker Build
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Code Analysis (javascript)
🔇 Additional comments (5)
types/Options/index.d.ts (1)
125-125: LGTM! TypeScript declaration is correct.The TypeScript declaration for
skipVerifyServerUrlfollows the established pattern for optional boolean properties in theParseServerOptionsinterface.src/Options/index.js (1)
60-64: LGTM! Flow interface declaration is correct.The Flow type declaration and documentation for
skipVerifyServerUrlare properly formatted and match the patterns used throughout this file.Note: The Biome static analysis error about the
?token is a false positive—Biome is not configured to parse Flow syntax.src/ParseServer.ts (2)
299-306: LGTM! Option destructuring is correct.The
skipVerifyServerUrloption is properly destructured alongside other configuration options.
377-382: Address remaining PR objectives before merge
- No tests cover the new skipVerifyServerUrl option; add specs verifying that verifyServerUrl is skipped when this flag is true.
- External documentation (e.g. README.md or docs site) wasn’t updated; please add or update docs for this feature.
- The planned security check for server URL validation is missing; confirm and implement the required check.
- New Parse Error codes for the JS SDK have not been added; specify which codes and include them in a follow-up commit or PR.
src/Options/docs.js (1)
106-106: LGTM! Documentation follows established patterns.The JSDoc documentation for
skipVerifyServerUrlis properly formatted and aligns with the documentation in other files.Note: If the security warning suggestion for
src/Options/Definitions.jsis accepted, this documentation should be updated to match.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #9881 +/- ##
==========================================
- Coverage 93.02% 93.01% -0.01%
==========================================
Files 187 187
Lines 15161 15163 +2
Branches 176 177 +1
==========================================
+ Hits 14103 14104 +1
- Misses 1046 1047 +1
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I'm not against the new option, but I think the original bug is that See #9882 for how this may be fixed. Feel free to incorporate the changes into your PR, or I'll simply merge my PR and then we can merge yours with only the new option. |
@mtrezza it worked nice before but i seems that Daniel added the timeout to stabilize something, and even if we move the code elsewhere the timeout will still be a problem and introduce a latency of 1sec on each test, if developers use isolated test, the impact of perf is huge. |
|
@Moumouls Did you take a look at my PR? No timeout needed. It's clear why the timeout was added - because the verification should not actually happen on mounting but on startup. |
|
feel free to close this one @mtrezza if you feel your PR resolve the issue and the verifyUrl on stratup with expo backoff works. I think your approach is better than mine :) |
|
@Moumouls sorry, I was just proposing that my PR may solve the root cause. Did you try it out? I've merged #9882 so you can use the alpha release. If you want to add the option to disable the verification, feel free to rebase and we can merge. However, the issue you originally opened this PR for should be solved with #9882. |
|
i think the option is still useful @mtrezza i'll refresh this PR :) |
…moumouls/allow-to-skip-verify-server-url # Conflicts: # src/ParseServer.ts
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Options/index.js (1)
60-64: LGTM! Clear documentation and correct implementation.The new option is well-documented with clear use cases and follows the established patterns in the codebase. The safe default (
false) ensures verification runs unless explicitly disabled.Note on static analysis: The Biome error flagging the
?at line 64 is a false positive—?booleanis valid Flow syntax for an optional boolean type.Optional security consideration: While the documentation is clear, you might consider adding a brief note emphasizing that when skipping verification, operators should manually ensure
serverURLis correctly configured, as skipping the check could mask misconfigurations that would otherwise surface early.Consider this optional enhancement to the documentation:
- /* Set to `true` to skip the server URL verification on startup. This can be useful in environments where the server URL is not accessible from the server itself, such as when running behind a firewall, in certain containerized environments, or in test environments like Jest where the verification may cause issues or unnecessary delays during test execution. + /* Set to `true` to skip the server URL verification on startup. This can be useful in environments where the server URL is not accessible from the server itself, such as when running behind a firewall, in certain containerized environments, or in test environments like Jest where the verification may cause issues or unnecessary delays during test execution. + <br><br> + ⚠️ When skipping verification, ensure that `serverURL` is correctly configured, as misconfigurations will not be detected automatically. <br><br> Default is `false`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Options/Definitions.js(1 hunks)src/Options/docs.js(1 hunks)src/Options/index.js(1 hunks)src/ParseServer.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Options/docs.js
- src/ParseServer.ts
- src/Options/Definitions.js
🧰 Additional context used
🪛 Biome (2.1.2)
src/Options/index.js
[error] 64-64: Expected a statement but instead found '?'.
Expected a statement here.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: Node 20
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: Node 18
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: Redis Cache
- GitHub Check: Docker Build
mtrezza
changed the title
verifyServerUrl to disable server URL verification on server launch
mtrezza
changed the title
verifyServerUrl to disable server URL verification on server launchverifyServerUrl to disable server URL verification on server launch
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
|
@Moumouls I've made some changes to the docs here, with your insights that these 2 features require PS to call itself:
I think with that it looks good to merge, what do you think? |
# [8.3.0-alpha.12](8.3.0-alpha.11...8.3.0-alpha.12) (2025-10-25) ### Features * Add Parse Server option `verifyServerUrl` to disable server URL verification on server launch ([#9881](#9881)) ([b298ccc](b298ccc))
|
🎉 This change has been released in version 8.3.0-alpha.12 |
3 participants
Pull Request
Issue
Adds a new Parse Server option skipVerifyServerUrl that allows skipping the automatic server URL verification that occurs on the mount event.
This option is useful in environments where the server URL is not accessible from the server itself:
Closes: FILL_THIS_OUT
Approach
Tasks
Summary by CodeRabbit
New Features
Documentation