Skip to content

Conversation

@Niccari
Copy link
Contributor

@Niccari Niccari commented Oct 15, 2025

Pull Request

Issue

Closes: #2753

Approach

When parsing response headers in RESTController, the header key check is no longer performed if there are no headers.

To ensure this issue can be detected, the unit test mockFetch was updated to throw an exception—just like the actual runtime—when an empty string is passed to headers.has.

Tasks

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

Summary by CodeRabbit

  • Bug Fixes

    • Avoid checking empty header names when reading response headers, reducing errors and improving stability when handling exposed headers.
  • Tests

    • Test helpers now raise a clear TypeError for empty header names.
    • Added tests to confirm AJAX requests succeed when the exposed-headers value is missing or empty.

@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 15, 2025

🚀 Thanks for opening this pull request!

@parseplatformorg
Copy link
Contributor

parseplatformorg commented Oct 15, 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 15, 2025

📝 Walkthrough

Walkthrough

Adds a truthy guard before calling response.headers.has when collecting exposed headers, updates mock Headers.has to throw on empty header names, and adds tests ensuring AJAX resolves when access-control-expose-headers is missing or empty. No public API changes.

Changes

Cohort / File(s) Summary of Changes
Response header handling guard
src/RESTController.ts
Skip calling response.headers.has(header) when header is falsy; preserve behavior for non-empty headers.
Test helper header validation
src/__tests__/test_helpers/mockFetch.js
Headers.has now throws TypeError for empty header names and otherwise returns existence as before.
Tests for empty/missing expose-headers
src/__tests__/RESTController-test.js
Added tests that mock 200 responses to ensure AJAX resolves when access-control-expose-headers is missing or an empty string.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant RESTController
  participant FetchAPI as Fetch API
  participant Headers

  Client->>RESTController: execute request
  RESTController->>FetchAPI: fetch(url, options)
  FetchAPI-->>RESTController: Response (headers, body)

  rect rgba(200,230,255,0.3)
    note right of RESTController: Collect exposed response headers
    loop for each header in exposeHeaders
      alt header is non-empty
        RESTController->>Headers: has(header)
        Headers-->>RESTController: boolean
        opt if true
          RESTController->>RESTController: record header value
        end
      else header is empty or falsy
        RESTController-->>RESTController: skip has() call
      end
    end
  end

  RESTController-->>Client: resolve with data
Loading
sequenceDiagram
  autonumber
  participant Test as Test Code
  participant MockHeaders as Mock Headers.has

  Test->>MockHeaders: has(headerName)
  alt headerName === ""
    MockHeaders-->>Test: throw TypeError("Headers.has: \"\" is an invalid header name.")
  else non-empty
    MockHeaders-->>Test: return headerName in headers
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review the falsy-check change in src/RESTController.ts for edge cases (e.g., whitespace-only header names).
  • Validate mock behavior in src/__tests__/test_helpers/mockFetch.js aligns with platform Headers.has semantics.
  • Confirm the two new tests in src/__tests__/RESTController-test.js correctly simulate missing and empty access-control-expose-headers.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "fix: Parse.Query throws error when no response header" accurately reflects the primary change in this PR. The title clearly indicates that the fix addresses an issue where Parse.Query throws an error related to response headers. While the specific detail that the error occurs when headers.has is called with an empty/invalid header name is not explicitly stated, the title is sufficiently clear and specific enough that a teammate can understand the main purpose—fixing a Parse.Query error triggered by problematic header handling. The title is directly related to the core issue being resolved (#2753).
Linked Issues Check ✅ Passed The code changes directly address the requirements of issue #2753. The root cause identified in the issue—that RESTController calls headers.has with an empty/invalid header name—is fixed by adding a truthy check on the header before calling headers.has in RESTController.ts. The mockFetch utility was updated to throw a TypeError for empty strings, matching actual runtime behavior and enabling proper test coverage. Two new tests were added to verify that AJAX requests resolve without error when the access-control-expose-headers header is missing or empty, directly validating that the fix resolves the reported issue where Parse.query() would throw an error.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7dd132 and 1854232.

📒 Files selected for processing (1)
  • src/__tests__/test_helpers/mockFetch.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/__tests__/test_helpers/mockFetch.js (1)
src/__tests__/RESTController-test.js (1)
  • headers (29-33)
🔇 Additional comments (1)
src/__tests__/test_helpers/mockFetch.js (1)

28-33: LGTM! Mock now correctly validates empty header names.

The implementation properly throws a TypeError when headers.has is called with an empty string, matching real browser behavior. This defensive check ensures that the fix in RESTController (guarding against empty header names) is properly tested and prevents future regressions.

The error message format matches the reviewer's suggestion from past comments and is descriptive.


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.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 15, 2025
rwblackburn added a commit to Blackburn-Labs/proem-ui that referenced this pull request Oct 15, 2025
Copy link

@kinetifex kinetifex left a comment

Choose a reason for hiding this comment

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

Encountered this also

@mtrezza
Copy link
Member

mtrezza commented Oct 24, 2025

@Niccari could you add a test?

@codecov
Copy link

codecov bot commented Oct 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.88%. Comparing base (22b0bab) to head (1854232).
⚠️ Report is 2 commits behind head on alpha.

Additional details and impacted files
@@           Coverage Diff           @@
##            alpha    #2754   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files          64       64           
  Lines        6220     6220           
  Branches     1488     1472   -16     
=======================================
  Hits         6213     6213           
  Misses          7        7           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 25, 2025
@Niccari
Copy link
Contributor Author

Niccari commented Oct 25, 2025

@Niccari could you add a test?

OK, I've added an explicit test at 4397cb1.

Note that the previous mockFetch did not throw an exception if the value of header.has was empty, but commit 5359aa8 changed it to throw an exception just like the actual runtime.

has: header => headers[header] !== undefined,
has: header => {
if (header === '') {
throw new TypeError('Headers.has: "" is an invalid header name.');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
throw new TypeError('Headers.has: "" is an invalid header name.');
throw new TypeError('Invalid empty header name.');

Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
@mtrezza mtrezza changed the title fix: Parse.query() raises TypeError Failed to execute 'has' on 'Headers': Invalid name when no response header fix: Parse.Query throws error when no response header Oct 25, 2025
@mtrezza
Copy link
Member

mtrezza commented Oct 25, 2025

Looks good, but waiting for more details in #2753 (comment) for proper changelog entry.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 25, 2025
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
@mtrezza
Copy link
Member

mtrezza commented Oct 25, 2025

Thanks for the PR! Waiting for CI to pass...

@mtrezza mtrezza merged commit 492de3e into parse-community:alpha Oct 25, 2025
13 checks passed
parseplatformorg pushed a commit that referenced this pull request Oct 25, 2025
## [7.0.2-alpha.1](7.0.1...7.0.2-alpha.1) (2025-10-25)

### Bug Fixes

* `Parse.Query` throws error when no response header ([#2754](#2754)) ([492de3e](492de3e))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.0.2-alpha.1

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Oct 25, 2025
parseplatformorg pushed a commit that referenced this pull request Oct 25, 2025
## [7.0.2](7.0.1...7.0.2) (2025-10-25)

### Bug Fixes

* `Parse.Query` throws error when no response header ([#2754](#2754)) ([492de3e](492de3e))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:released Released as stable version state:released-alpha Released as alpha version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parse.query() raisesUncaught (in promise) Error: Failed to execute 'has' on 'Headers': Invalid name

4 participants