Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion - debug_backtrace() replacements #25

Open
langabi opened this issue Sep 26, 2021 · 1 comment
Open

Suggestion - debug_backtrace() replacements #25

langabi opened this issue Sep 26, 2021 · 1 comment

Comments

@langabi
Copy link
Contributor

langabi commented Sep 26, 2021

Suggestion, for utility functions - I put them in Trace.php. Almost-dropin replacements for PHP debug_backtrace() and debug_print_backtrace(), useful for retrieving backtrace without having to throw a fatal exception. Happy to create as pull request, but quite possible I missed something vital in writing these.

    /**
     * Replacement for debug_print_backtrace() PHP function. Returns the current stack as a string
     * 
     * Call as `yield Trace::debug_print_backtrace();`
     */
    public static function debug_print_backtrace() : \Generator
    {
        /** @var Trace $trace */
        $trace = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null;
        $e = new Exception();
        $trace->updateStackTrace($e);
        return $e->getTraceAsString();
    }

    /**
     * Replacement for debug_backtrace() PHP function. Returns the current stack as an array
     * 
     * Call as `$backtrace = yield Trace::debug_backtrace();`
     */
    public static function debug_backtrace() : \Generator
    {
        /** @var Trace $trace */
        $trace = \class_exists(\Recoil\Dev\Instrumentation\Trace::class) ? yield \Recoil\Dev\Instrumentation\Trace::install() : null;
        $e = new Exception();
        $trace->updateStackTrace($e);
        return $e->getTrace();
    }
@jmalloc
Copy link
Member

jmalloc commented Sep 26, 2021

These definitely seem like useful functions to have available, thanks! Some thoughts below - mostly for myself - but if you did wan't to attempt a PR there's some decisions to be made first :)

I'm reluctant to overload the purpose of Recoil\Dev\Instrumentation\Trace class to be both the implementation of Recoil\Api\StrandTrace and part of the public API called by users.

Perhaps instead these methods could be placed within a new Recoil\Dev\RecoilDev type, analogous to Recoil\Recoil but for operations that are only available when recoil/dev is installed:

use Recoil\Dev\RecoilDev;
use Generator as Coroutine;

function someCoroutine() : Coroutine {
    yield RecoilDev::print_backtrace();
}

Pros:

  • Simple to implement, no kernel changes

Cons:

  • It might lead to code that passes tests but fails in production if a call to one of these operations is accidentally left in the code. My thinking here is that recoil/dev is usually installed via require-dev, and hence is typically available when running tests, but unavailable in production where we'd typically install with composer install --no-dev.

For that reason it might be better to place these methods directly in Recoil\Recoil, and simply have them forward to the built-in functions if recoil/dev is unavailable.

Pros:

  • Methods are always available regardless of how composer install is run
  • API docs in the same place as everything else

Cons:

  • BC breaking change to recoil/api which will break existing kernel implementations (though this likely only affects me)
  • The kernel would need to a grow a mechanism that allows recoil/dev to intercept and change the behavior of these methods so they no longer forward to the built-in functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants