silverstripe / silverstripe-graphql

Serves Silverstripe data as GraphQL representations
BSD 3-Clause "New" or "Revised" License
52 stars 61 forks source link

FIX Don't error silently when arguments are enabled #523

Closed GuySartorelli closed 1 year ago

GuySartorelli commented 1 year ago

If PHP is configured to include arguments, the json_encode($result) call in graphql's error handling was hitting a recursion error (specifically found while validating https://github.com/silverstripe/silverstripe-graphql/issues/500).

This PR avoids that problem entirely by treating the backtrace the same as we do everywhere else in framework. It also ensures that arguments are filtered appropriately to avoid leaking sensitive information.

Issue

GuySartorelli commented 1 year ago

Rather than doing this with a real exception, I'm just going to pass in a mocked exception trace into the new private method. It's a good idea to validate that method in a unit test or 2 anyway to ensure it stays up to date with the Backtrace class.

GuySartorelli commented 1 year ago

Added backtrace test

GuySartorelli commented 1 year ago

CI failures are because there's no version of silverstripe/installer with constraints that are happy with the 4.2.x-dev constraint for this module.