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

feat: increase backtrace frame limit #233

Merged
merged 1 commit into from
Jul 27, 2023
Merged

Conversation

jaulz
Copy link
Contributor

@jaulz jaulz commented Apr 6, 2023

Currently, the limit of backtrace frames is set to 10 and cannot be changed. It would be great if it would be possible to increase the limit for custom design patterns (e.g., I would like to use it in combination with Lighthouse). Anyway, while debugging the same I was wondering whether the first 9 frames could be skipped because those all belong to laravel or laravel-actions and thus the time could rather be spent on real potential matches.

@jaulz
Copy link
Contributor Author

jaulz commented Jul 27, 2023

@lorisleiva sorry to disturb but would you be able to give some feedback on this? 😊

@lorisleiva
Copy link
Owner

Hi there, sorry for the late reply. I'm a bit worried about skipping backtrace frame we don't control in case something changed in the framework but I'd be happy to skip the ones that are purely from the package as we control that part. Would you be able to show me the frames we'd be skipping?

Also you mention in your OP that you'd like the limit to be parameterised. I can see the limit as an argument in this method but perhaps you'd like this to be accessible at a higher level somewhere? Is this what you meant?

@jaulz
Copy link
Contributor Author

jaulz commented Jul 27, 2023

@lorisleiva thanks for the quick response! 😊 So this is the stacktrace:
image

Essentially there are only 2 frames from your library so it would only be a minor performance gain. However, I just pushed an update that reduces the frames anyway and also adds a method to change the limit. The limit can be adapted like this:

    $this->app->extend(ActionManager::class, function (ActionManager $actionManager) {
      $actionManager->setBacktraceLimit(20);

      return $actionManager;
    });

What do you think?

@jaulz jaulz changed the title fix: increase backtrace frame limit feat: increase backtrace frame limit Jul 27, 2023
@jaulz
Copy link
Contributor Author

jaulz commented Jul 27, 2023

For some reason the commit does not appear here but it's actually pushed to https://github.com/jaulz/laravel-actions/tree/patch-1 🤔

Nevermind, it updated after pushing a second commit 😊

feat: add configurable backtrace limit

style: add new lines

fix: rename variable
Copy link
Owner

@lorisleiva lorisleiva left a comment

Choose a reason for hiding this comment

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

Thanks!

@lorisleiva lorisleiva merged commit 1a25bb3 into lorisleiva:main Jul 27, 2023
8 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.

2 participants