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

Cherry-picking/Rebase: error softly when unexpected null state exists #13889

Merged
merged 1 commit into from Feb 16, 2022

Conversation

tidy-dev
Copy link
Contributor

Description

TLDR; We get to an unexpected null state in cherry-picking/rebase and that crashes the app (so far unreproducible and uncommon). This pr aims to error out more gracefully. The app should be returned to a usable non-cherry-picking/rebase state.

We have received errors indicating the application is crashing because the application is attempting to update the multiCommitOperation state when that state is null. We have received this error for rebase and cherry-picking. We have been unable to reproduce these errors.

Cannot update a null state.
app/src/lib/stores/repository-state-cache.ts:141:15
Object.update (app/src/lib/stores/repository-state-cache.ts:45:23)
Object.updateMultiCommitOperationState (app/src/lib/stores/repository-state-cache.ts:138:10)
app/src/lib/stores/app-store.ts:6137:33
a.<anonymous> (app/src/lib/git/rebase.ts:335:11)
a.emit (events.js:315:20)
addChunk (_stream_readable.js:295:12)
readableAddChunk (_stream_readable.js:271:9)
a.Readable.push (_stream_readable.js:212:10)
a.Transform.push (_stream_transform.js:152:32)
Cannot update a null state.
app/src/lib/stores/repository-state-cache.ts:141:15
Object.update (app/src/lib/stores/repository-state-cache.ts:45:23)
Object.updateMultiCommitOperationState (app/src/lib/stores/repository-state-cache.ts:138:10)
app/src/lib/stores/app-store.ts:6137:33
a.<anonymous> (app/src/lib/git/cherry-pick.ts:120:11)
a.emit (events.js:315:20)
addChunk (_stream_readable.js:295:12)
readableAddChunk (_stream_readable.js:271:9)
a.Readable.push (_stream_readable.js:212:10)
a.Transform.push (_stream_transform.js:152:32)

This PR aims to prevent the application from crashing and to silently send us the error along with a slight bit more data to confirm where in the progress this is happening. My assumption is that this should be safe and not leave the user in a bad application state, because, if the multiCommitOperationState is null, that would indicate that somehow the user already reached the end of the operation and the app should return to a base non rebase/cherry-pick state.

Looking at the line numbers, it would seem that it is the progress callback from the git operations firing after the multi commit operation has been set to null. My best guess is that a user is jumping between Desktop and command line to complete the rebase or cherry-pick and the app is moving between detecting state and working through the state? Other possible scenarios are some combination of error at the same time as the callback fires as we clear the state on errors. Either way seems like a timing/combination of things problem and is why it is difficult to pin point/reproduce.

Release notes

Notes: [Fixed] App no longer crashes sometimes when rebasing and cherry-picking.

Copy link
Member

@sergiou87 sergiou87 left a comment

Choose a reason for hiding this comment

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

I like the approach, hopefully getting that info in the non-fatal error will help us debugging the issue 🤞

@tidy-dev tidy-dev merged commit ed8ac70 into development Feb 16, 2022
@tidy-dev tidy-dev deleted the multi-commit-error-softely-on-null-state branch February 16, 2022 13:32
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.

None yet

2 participants