HarperFast / HarperFast/harper

Chained-condition error paths: second `&=` leg unparseable, `|=` chain → 500 with wrong message, parseQuery TypeError without query arg

Open
#2,435 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## Summary

Three related error-path defects around REST chained conditions (`&=` / `|=`). None produces silent wrong data (unlike the chained-value coercion bug filed separately) — these fail loudly but wrongly.

### 1. A second chained leg can never be parsed

`skiLengths=ge=170&=le=180&=ge=171` fails to parse with the unrelated error `attribute must be specified before equality comparator`.

Cause: after each token the parser picks its tokenizer by `parser = attribute ? VALUE_PARSER : QUERY_PARSER` (`resources/search.ts:1571-1574`). A chained leg sets `attribute = ''` (falsy), so parsing continues with `QUERY_PARSER`, whose operator class cannot produce `&=` as one token (`VALUE_PARSER`'s `[&|=]*` run can — that is the only reason the *first* `&=` works, since at that point `attribute` is the real attribute name and truthy). The split `&` + `=` then trips the equality-comparator error.

Consequence: the planner's `Multiple chained conditions are not currently supported` error (`resources/Table.ts:3530`) is unreachable via REST, and the user gets a misleading message for a syntactically plausible query.

### 2. `|=` chains throw a plain 500 with the wrong message

`skiLengths=ge=175|=le=180` parses fine (one chained condition, `operator: 'or'`), then `prepareConditions` throws `Error('Multiple chained conditions are not currently supported')` (`resources/Table.ts:3530`) — a plain `Error` (→ 500), and the message is wrong twice over: there is only **one** chained condition, and the unsupported part is the `or` operator. Should be a 4xx `ClientError` saying or-chaining is unsupported.

### 3. `parseQuery(str)` without a query object crashes on any parse error

`parseQuery` reads `query.parseErrorMessage` where `query` may be `undefined` — `resources/search.ts:1343`:

```ts
(currentQuery as any).parseError = new SyntaxViolation((query as any).parseErrorMessage);
```

should read `currentQuery.parseErrorMessage`. With no `query` argument, any recorded parse error becomes `TypeError: Cannot read properties of undefined (reading 'parseErrorMessage')` instead of the intended `SyntaxViolation`. Production REST always passes a query object (`RequestTarget.ts:105`), so this only hits direct callers of the public `Resource.parseQuery(search)` static and internal tooling.

## Reproductions

All three reproduce with `parseQuery` + `Table.search` on `main` (a21910cff); pinning tests are in the linked verification PR.

## Severity

Error-path/UX only: loud failures, wrong shape (misleading message, 500 instead of 400, TypeError instead of SyntaxViolation). No wrong result sets.

Contributor guide

Open the contributing guide

Research direction

Run the parseQuery + Table.search reproductions, then read resources/search.ts around lines 1343 and 1571-1574, resources/Table.ts:3530, and RequestTarget.ts:105. Done means chained conditions produce the intended client error, |= reports unsupported or-chaining as a 4xx, and parseQuery without a query argument preserves SyntaxViolation instead of throwing TypeError.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js, typescript
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
63/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.