Stack traces are missing variadic and not declared arguments

[xwillq]

trafficstars

How do you use Sentry?

Self-hosted / on-premises

SDK version

4.0.1

Steps to reproduce

1. Create function with variadic parameters or without parameters. Ex.:

function variadic(...$params) {...}
function noParams() {...}

2. Call this function with some arguments.
3. Throw an exception inside that function.

Expected result

Sentry shows all passed arguments in stack trace.

Actual result

Sentry shows only first argument for variadic function and no arguments for function without parameters. With this code:

function variadic(...$params)
{
    noParams($params);
}

function noParams()
{
    throw new \Exception();
}

Artisan::command('debug', function () {
    variadic('a', 1, [123]);
});

I get this stack trace in Sentry:

[xwillq]

I believe the culprit is \Sentry\FrameBuilder::getFunctionArgumentValues. It only takes values passed for defined parameters based on their position, which means it completely ignores parameters that weren't defined or parameters that have multiple values.

[xwillq]

Also, \Sentry\FrameBuilder::getFunctionArgumentValues ignores nulls, since it uses isset() to check if argument was passed to the function, and it doesn't use default values if nothing was passed to the function. This might not be an issue depending on what's the purpose in sending parameters to sentry. If it's to show the complete picture, that it probably would be useful to send those. But if it is supposed to give just enough information to reconstruct the rest from the code, then sending arguments that was actually passed would be enough.

[cleptric]

I haven't tested this yet, but how are variadic parameters reflected in a native stack trace?

[xwillq]

All ways to pass parameters to function look the same way in PHP 7.4. Functions' code:

function regularParams(string $s, int $i, array $a)
{
    variadic($s, $i, $a);
}

function variadic(...$params)
{
    noParams(...$params);
}

function noParams()
{
    throw new \Exception();
}

Artisan::command('debug', function () {
    try {
        regularParams('a', 1, [123]);
    } catch (\Throwable $e) {
        var_dump(array_slice($e->getTrace(), 0, 3));
    }
});

var_dump output:

array(3) {
  [0] =>
  array(4) {
    'file' =>
    string(23) "/app/routes/console.php"
    'line' =>
    int(29)
    'function' =>
    string(8) "noParams"
    'args' =>
    array(3) {
      [0] =>
      string(1) "a"
      [1] =>
      int(1)
      [2] =>
      array(1) {
        ...
      }
    }
  }
  [1] =>
  array(4) {
    'file' =>
    string(23) "/app/routes/console.php"
    'line' =>
    int(24)
    'function' =>
    string(8) "variadic"
    'args' =>
    array(3) {
      [0] =>
      string(1) "a"
      [1] =>
      int(1)
      [2] =>
      array(1) {
        ...
      }
    }
  }
  [2] =>
  array(4) {
    'file' =>
    string(23) "/app/routes/console.php"
    'line' =>
    int(39)
    'function' =>
    string(13) "regularParams"
    'args' =>
    array(3) {
      [0] =>
      string(1) "a"
      [1] =>
      int(1)
      [2] =>
      array(1) {
        ...
      }
    }
  }
}

[cleptric]

We use the args to add the variables to the Sentry stack trace. Not sure if we can improve this much.

[xwillq]

args has all required info. I made quick and maybe dirty fix.

private function getFunctionArgumentValues(\ReflectionFunctionAbstract $reflectionFunction, array $backtraceFrameArgs): array
{
    $argumentValues = [];

    foreach ($reflectionFunction->getParameters() as $reflectionParameter) {
        if (!empty($backtraceFrameArgs)) {
            // Since getParameters() returns parameters in order they were
            // declared, we can just remove first argument from array to
            // get value for first parameter.
            $value = array_shift($backtraceFrameArgs);
        } else {
            // No more args, we can get out of the loop.
            // (or we can continue and handle default values here)
            break;
        }

        $argumentValues[$reflectionParameter->getName()] = $value;
    }

    $lastParameter = last($reflectionFunction->getParameters()) ?: null;
    if ($lastParameter !== null && $lastParameter->isVariadic()) {
        $lastParameterName = $lastParameter->getName();
        // Since variadic parameters available as arrays inside of functions,
        // we can wrap passed value into array so Sentry would represent it
        // the same way we would see it in code.
        $argumentValues[$lastParameterName] = [
            $argumentValues[$lastParameterName],
        ];
    }

    if (!empty($backtraceFrameArgs)) {
        if ($lastParameter !== null && $lastParameter->isVariadic()) {
            $lastParameterName = $lastParameter->getName();
            // If we still have args and last parameter is variadic,
            // merge its value with remaining args.
            $argumentValues[$lastParameterName] = array_merge(
                $argumentValues[$lastParameterName],
                $backtraceFrameArgs,
            );
        } else {
            // If last parameter is not variadic, it means we passed undefined
            // parameters into function. They are probably used with
            // func_get_args(), or this could be a bug. In any case, it
            // would be useful to see them in Sentry.
            $parameterPosition = count($argumentValues) + 1;
            foreach ($backtraceFrameArgs as $argument) {
                $argumentValues['param' . $parameterPosition] = $argument;
                $parameterPosition += 1;
            }
        }
    }

    return $argumentValues;
}

I'm not sure about performance and all the edge cases, but it solves the problem. Here is the before and after for code snipper from my last comment.

Before

Stack frame for variadic function contains only one variable, for function without arguments it doesn't contain anything.

After

Both stack frames contain all passed variables.

I can open a merge request if you think this solution is good enough or could be used as a start.

[cleptric]

Oh, if you are okay with just having the values, sure, please open a PR (including tests 😬). I thought you wanted the argument names as well.

[xwillq]

The variadic parameter has a name, params, though it was a confusing one in this context 😅. For function without parameters at all we cannot do much, so I took the same name FrameBuilder uses when it cannot create reflection object. I think thats the best we could do for this two cases.