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

[check-log] Ignore broken/unexpected json on reading state #302

Merged
merged 5 commits into from
Oct 29, 2019

Conversation

astj
Copy link
Contributor

@astj astj commented Oct 22, 2019

I'm not sure this is the best approach, but worth consider…

@@ -444,6 +451,8 @@ func getStateFile(stateDir, f string, args []string) string {
)
}

var errValidStateFileNotFound = fmt.Errorf("state file not found, or corrupted")

func getBytesToSkip(f string) (int64, error) {
state, err := loadState(f)
Copy link
Member

Choose a reason for hiding this comment

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

If state file is corrupted, loadState() will return nil, nil.
Is it intended behavior that getBytesToSkipOld will be called to try to read old state file in this case?

Copy link
Contributor Author

@astj astj Oct 25, 2019

Choose a reason for hiding this comment

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

I thought the behavior is not the best but acceptable.
But finally I decided not to read old state file the case faeb49f

if !opts.NoState {
s, err := getBytesToSkip(stateFile)
if err != nil {
return 0, 0, "", err
if err != errValidStateFileNotFound {
Copy link
Member

Choose a reason for hiding this comment

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

getBytesToSkip can return errValidStateFileNotFound only if getBytesToSkipOld is called.

@astj astj merged commit 86ed8ec into master Oct 29, 2019
@astj astj deleted the check-log-ignore-broken-json branch October 29, 2019 15:06
@lufia lufia mentioned this pull request Nov 21, 2019
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