Skip to content

Conversation

Niccari
Copy link

@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
    • Resolved an error when processing response headers by skipping checks for empty header names, improving stability when reading exposed headers.
  • Tests
    • Enhanced test utilities to validate header names and throw a clear error for empty values, aligning tests with expected runtime behavior.

Copy link

🚀 Thanks for opening this pull request!

@parseplatformorg
Copy link
Contributor

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.

Copy link

coderabbitai bot commented Oct 15, 2025

📝 Walkthrough

Walkthrough

Adds a truthy guard before calling Response.headers.has during exposed header collection in RESTController, preventing empty-name checks. Updates test mock Headers.has to throw TypeError on empty header names, aligning behavior with browser Fetch. No public API changes.

Changes

Cohort / File(s) Summary of Changes
Response header handling guard
src/RESTController.ts
Skips calling response.headers.has(header) when header is falsy; maintains existing behavior for non-empty headers.
Test helper header validation
src/__tests__/test_helpers/mockFetch.js
Headers.has now validates input; throws TypeError for empty header names, otherwise unchanged return behavior.

Sequence Diagram(s)

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

  Client->>RESTController: execute request
  RESTController->>Fetch: fetch(url, options)
  Fetch-->>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 has(header)
          RESTController->>RESTController: record header value
        end
      else header is empty
        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

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • mtrezza

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly specifies the bug being fixed by indicating that Parse.query() no longer raises the specified Headers.has TypeError when no response header is present, directly reflecting the main change in the PR. It is specific and immediately conveys the core issue addressed without introducing unrelated details.
Linked Issues Check ✅ Passed The changes directly address issue #2753 by adding a guard in RESTController to skip checking empty header names and updating mockFetch tests to throw on invalid header names, thereby preventing the Headers.has TypeError and satisfying the core requirement for successful queries.
Out of Scope Changes Check ✅ Passed All modifications are confined to the headers parsing logic in RESTController and related test mocks, with no unrelated or extraneous code changes present in the PR.
Description Check ✅ Passed The pull request description adheres to the repository’s template by including the Pull Request header with license and issue linking, a completed Issue section with the correct issue number, a clear Approach section describing the header parsing change, and a Tasks list showing tests and documentation updates.
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 7bec0fd and cced7d1.

📒 Files selected for processing (2)
  • src/RESTController.ts (1 hunks)
  • 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 (2)
src/RESTController.ts (1)

161-161: LGTM! Correct fix for the empty header name issue.

The truthy guard prevents calling response.headers.has('') when the split produces empty strings (e.g., from trailing commas or empty access-control-expose-headers), which would throw a TypeError in browsers. This is the minimal fix that addresses the reported issue.

src/__tests__/test_helpers/mockFetch.js (1)

28-33: LGTM! Test mock now matches browser behavior.

The updated mock correctly throws a TypeError when has() is called with an empty string, matching the actual Fetch API Headers behavior. This ensures tests will catch regressions of the issue fixed in RESTController.ts.


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.

rwblackburn added a commit to Blackburn-Labs/proem-ui that referenced this pull request Oct 15, 2025
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.

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

2 participants