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

Add Option to Disable Error Collection #615

Merged
merged 7 commits into from Jan 6, 2022

Conversation

BeeeQueue
Copy link
Contributor

I ran into the same(?) issue as #600 and found that commenting out https://github.com/cenk1cenk2/listr2/blob/master/src/lib/task-wrapper.ts#L58 fixed it based on the comments in the issue.

Closes #600(?)

@cenk1cenk2
Copy link
Collaborator

Thank you @BeeeQueue for this merge request, I would only change to enable this by default and merge it with your other request as a breaking change. I doubt that many people including me use this feature since it was rather fixed recently and causing more problems than it solves. Maybe we can just collect the error.message when collectErrors is set to false, so we do know something has gone wrong.

I will merge this is hopefully tomorrow.

@BeeeQueue
Copy link
Contributor Author

Maybe a better solution would be to have the setting be something like errorCollection: "full" | "minimal" | false"?

Then it can do the full copy on "full", just save Error.{message,stack} on "minimal", and nothing when false

@cenk1cenk2
Copy link
Collaborator

Yeah that would be even better. What should the default will be I guess minimal without copying the context for each error it would be very hard to have a memory leak.

@BeeeQueue
Copy link
Contributor Author

BeeeQueue commented Jan 5, 2022

Looking at the function again it would be better to just save the error and save the extra info (ctx, task) if it's not "minimal".

Then the question becomes how do we know which step actually errored? With no task or context in the error it becomes quite hard to tell in a big list of tasks.

I added something called currentPath to Task, which is all the task's parent's titles, which we can then use to show which task crashed.

image

@cenk1cenk2 cenk1cenk2 merged commit 357f34b into listr2:master Jan 6, 2022
@cenk1cenk2
Copy link
Collaborator

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@BeeeQueue BeeeQueue deleted the disable-error-collection branch January 6, 2022 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version 3.13.2 and later results in application crash and memory leaks
2 participants