Skip to content

Throw client-safe errors for malformed date literals - #2788

Merged
spawnia merged 1 commit into
nuwave:masterfrom
arunarw:fix-date-literal-client-safe
Sep 2, 2026
Merged

Throw client-safe errors for malformed date literals#2788
spawnia merged 1 commit into
nuwave:masterfrom
arunarw:fix-date-literal-client-safe

Conversation

@arunarw

@arunarw arunarw commented Sep 2, 2026

Copy link
Copy Markdown
Contributor
  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md (skip for docs-only changes)

Changes

A malformed date is handled differently depending on whether it arrives as a query literal or as a variable. As a variable it is a client error. As a literal it is classified as a server fault: the message is masked as "Internal server error" and the exception is passed to the application's error reporter.

parseValue()tryParsingDate() rethrows without a previous:

throw new $exceptionClass($exception->getMessage()); // previous = null → client-safe

parseLiteral() preserved it:

throw Error::createLocatedError($exception, $valueNode); // previous = InvalidFormatException

Carbon\Exceptions\InvalidFormatException does not implement ClientAware, so Error::__construct sets isClientSafe = false. FormattedError then masks the message, and ReportingErrorHandler forwards the exception to ExceptionHandler::report().

This affects Date, DateTime, DateTimeTz and DateTimeUtc, which all inherit parseLiteral from DateScalar.

The fix throws a client-safe error while keeping $valueNode, so locations is unchanged:

throw new Error($exception->getMessage(), $valueNode);

The added test lives in DateScalarTestBase, so it covers all four scalars. It fails on all four before the change:

4) Tests\Unit\Schema\Types\Scalars\DateTimeUtcTest::testThrowsClientSafeErrorIfParseLiteralInvalidDate
A malformed date literal is client misuse, so it must not be reported as a server error.
Failed asserting that false is true.

Tests: 4, Assertions: 8, Failures: 4.

The existing suite already covered parseValue with an invalid date and parseLiteral with a non-string node, but not parseLiteral with a malformed date string — which is why this went unnoticed.

We hit this on a Date query argument in production: roughly 6,200 reported exceptions from one client sending YYYY/MM/DD, each surfacing as a masked "Internal server error" rather than a usable validation message.

Breaking changes

None. Malformed literals still throw an Error with the same message and locations; only isClientSafe changes, so the error is no longer masked or reported. Valid values, parseValue and serialize are untouched.

@arunarw
arunarw force-pushed the fix-date-literal-client-safe branch from 38d18ee to f64f658 Compare September 2, 2026 09:35
DateScalar::parseLiteral passed the Carbon exception to
Error::createLocatedError, which keeps it as the error's previous.
Since InvalidFormatException is not ClientAware, that marked the error
as non-client-safe: the message was masked as "Internal server error"
and the exception was handed to the application's error reporter.

The variable path already rethrows without a previous via
tryParsingDate, so the same malformed value was a client error as a
variable but a server fault as a literal. Bring literals in line.
@arunarw
arunarw force-pushed the fix-date-literal-client-safe branch from f64f658 to 32e189e Compare September 2, 2026 12:04
@spawnia
spawnia requested a balanced review from Copilot September 2, 2026 12:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The targeted implementation preserves locations, corrects error classification, and includes appropriate shared test coverage.

Pull request overview

Makes malformed date literals client-safe so GraphQL returns useful validation errors without server-side reporting.

Changes:

  • Wraps parsing failures in client-safe GraphQL errors while preserving source locations.
  • Adds coverage for all four date scalar types.
  • Documents the fix in the changelog.
File summaries
File Description
src/Schema/Types/Scalars/DateScalar.php Makes malformed literal errors client-safe.
tests/Unit/Schema/Types/Scalars/DateScalarTestBase.php Verifies client-safe errors across date scalars.
CHANGELOG.md Records the corrected behavior.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@spawnia
spawnia merged commit 9963116 into nuwave:master Sep 2, 2026
90 checks passed
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.

3 participants