Skip to content

Conversation

@modime
Copy link
Contributor

@modime modime commented Oct 27, 2025

Pull Request

This Pull Request fixes a line of code that existed in Cloud.ts in v5.3.0 but was removed in v6.0.0.

This removal added a breaking change of the code with different behaviour when the options parameter is null.

Relates to #2622 and #2623

Issue

Closes: #2622

Approach

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)

Summary by CodeRabbit

  • Bug Fixes
    • Improved stability by ensuring optional configuration parameters are properly initialized during operations.

Marcus Olsson and others added 9 commits May 25, 2025 01:36
Signed-off-by: Manuel <[email protected]>
Signed-off-by: Manuel <[email protected]>
Ensures that the `options` argument in the `run` function is initialized to an empty object if it is undefined, preventing potential errors when accessing properties of `options` later in the function.

Signed-off-by: Marcus Olsson <[email protected]>
@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 27, 2025

🚀 Thanks for opening this pull request!

@parseplatformorg
Copy link
Contributor

parseplatformorg commented Oct 27, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2025

📝 Walkthrough

Walkthrough

Fixes null handling in the Cloud.run options parameter by setting a default empty object when options is null or undefined, preventing downstream errors when null is passed instead of undefined.

Changes

Cohort / File(s) Summary
Options parameter initialization
src/Cloud.ts
Added default value assignment `options = options

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • mtrezza
  • kinetifex

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ⚠️ Warning The pull request description does not properly follow the required template structure. While the author provided useful context about the issue and the breaking change that was introduced, critical required fields remain unfilled: the "Closes" field still contains the placeholder text "FILL_THIS_OUT" instead of the actual issue number (#2622 as mentioned in the PR objectives), and the "Approach" section is empty except for the template comment. Although the author provided some substantive information in the opening paragraphs, they did not organize the description according to the template's required sections, leaving placeholder content unaddressed. The author should properly fill out the template by replacing "FILL_THIS_OUT" in the "Closes" field with "#2622" (the issue this PR addresses), and should remove the template comment placeholder from the "Approach" section and add a clear description of the changes made. The context already provided in the opening paragraphs could be moved into the Approach section, and the task checkboxes should be addressed to indicate whether tests or documentation were added.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues Check ✅ Passed Issue #2622 describes a bug where passing null as the options parameter to Parse.Cloud.run() causes a TypeError because _getRequestOptions lacks null-safe handling. The issue requests either returning an empty object or implementing a null check. The code change in this PR directly addresses this by setting options = options || {} in the Cloud.ts implementation, which provides a default empty object when options is falsy (including null and undefined), matching the expected behavior described in the issue.
Out of Scope Changes Check ✅ Passed The changeset includes modifications only to src/Cloud.ts, with the specific change being the addition of a default value for the options parameter. This change is directly aligned with resolving issue #2622, which requires null-safe handling of the options parameter. No additional unrelated files, refactoring, or scope creep is present in the changeset. The modification is focused, minimal, and entirely relevant to the stated objective.
Title Check ✅ Passed The PR title "Fix for options default value when missing" directly and accurately summarizes the main change in the pull request. The raw summary confirms the change introduces a default for options in the generic run function by setting options = options || {} when options is not provided, and the PR objectives clarify this restores functionality that existed in v5.3.0 but was removed in v6.0.0. The title is concise, clear, and specific enough for a teammate scanning history to understand the primary change without being vague or overly broad.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@modime modime changed the title Fix for options default value when missing Fix: Sets default value for missing options parameter Oct 27, 2025
Signed-off-by: Marcus Olsson <[email protected]>
@modime modime changed the title Fix: Sets default value for missing options parameter Fix: sets default value for missing options parameter Oct 27, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Cloud.ts (1)

126-126: Critical: Null-safety issue in startJob method.

Line 126 directly assigns options.returnStatus = true without checking if options is defined. If startJob is called without options (or with null), this will throw a TypeError, similar to the issue this PR fixes in the run method.

Add null-safety before accessing options:

  async startJob(name: string, data: any, options?: RequestOptions): Promise<string> {
    const RESTController = CoreManager.getRESTController();

    const payload = encode(data, true);
+   options = options ?? {};
    options.returnStatus = true;

    const response = await RESTController.request('POST', 'jobs/' + name, payload, options);
    return response._headers?.['X-Parse-Job-Status-Id'];
  },
🧹 Nitpick comments (1)
src/Cloud.ts (1)

49-49: Good fix; consider using nullish coalescing for precision.

The fix correctly addresses the reported TypeError when null is passed. However, || converts all falsy values (0, false, "", etc.) to {}, whereas nullish coalescing (??) only defaults null/undefined. Given the TypeScript signature, both work, but ?? is more precise and aligns with modern best practices.

Apply this diff to use nullish coalescing:

-  options = options || {};
+  options = options ?? {};
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7cc7a16 and 1e087b5.

📒 Files selected for processing (1)
  • src/Cloud.ts (1 hunks)
🔇 Additional comments (1)
src/Cloud.ts (1)

48-54: Verify test coverage for null options handling.

The PR objectives indicate that tests have not yet been added. Please ensure unit tests cover the scenario where null is explicitly passed as the options parameter to verify the fix works as expected.

Example test case to add:

// Test that null options doesn't throw TypeError
await Parse.Cloud.run('functionName', { data: 'value' }, null);

@modime modime closed this Oct 27, 2025
@modime modime deleted the fix/options-default-value-missing branch October 27, 2025 16:07
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.

Error when passing null as Parse.Cloud.run option

3 participants