`timetrace create record` panics because of nil pointer dereference

[dominikbraun]

There seems to be a bug in the timetrace create record command:

dominik.braun@Chefkoch3027 % timetrace create record web-store today 14:00 16:07
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x12c8acd]

goroutine 1 [running]:
github.com/dominikbraun/timetrace/core.collides(0x0, 0xed86d0240, 0x15f0860, 0xc0001363a8, 0xc00010d8d0, 0x0, 0xc00010dab0, 0x2, 0x2, 0x0)
	/home/circleci/project/core/timetrace.go:306 +0x14d
github.com/dominikbraun/timetrace/core.(*Timetrace).RecordCollides(0xc00014a5e0, 0x0, 0xed86d0240, 0x15f0860, 0xc0001363a8, 0xc00010d8d0, 0x0, 0x0, 0xed86d2004, 0x15f0860)
	/home/circleci/project/core/timetrace.go:295 +0x15d
github.com/dominikbraun/timetrace/cli.createRecordCommand.func1(0xc0001aca00, 0xc000128a80, 0x4, 0x4)
	/home/circleci/project/cli/create.go:95 +0x3fc
github.com/spf13/cobra.(*Command).execute(0xc0001aca00, 0xc000128a00, 0x4, 0x4, 0xc0001aca00, 0xc000128a00)
	/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:856 +0x2c2
github.com/spf13/cobra.(*Command).ExecuteC(0xc0001ac280, 0x14061b8, 0x7, 0xc0001ac280)
	/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:960 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
	/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:897
main.main()
	/home/circleci/project/main.go:24 +0x28a

The error occurs in timetrace.go:306, which is the collides function checking a value that is nil.

timetrace/core/timetrace.go

Lines 298 to 312 in 0c9984c

	func collides(toCheck Record, allRecords []*Record) bool {
	for _, rec := range allRecords {
	if rec.Start.Before(toCheck.Start) && rec.End.After(toCheck.Start) {
	return true
	} else if rec.Start.Before(*toCheck.End) && rec.End.After(*toCheck.End) {
	return true
	} else if toCheck.Start.Before(rec.Start) && toCheck.End.After(rec.Start) {
	return true
	} else if toCheck.Start.Before(*rec.End) && toCheck.End.After(*rec.End) {
	return true
	}
	}
	return false
	}

I suspect that rec.End is nil here because my latest record hasn't been stopped yet:

dominik.braun@Chefkoch3027 % timetrace list records today
+-----+------------------+-----------+---------+-----------+------------+
|  #  |       KEY        |  PROJECT  |  START  |    END    |  BILLABLE  |
+-----+------------------+-----------+---------+-----------+------------+
|   1 | 2021-06-29-16-08 | web-store | 16:08   | ---       | no         |
|   2 | 2021-06-29-09-05 | web-store | 09:05   | 13:02     | no         |
+-----+------------------+-----------+---------+-----------+------------+
|                                                TOTAL:    | 3H 57MIN   |
+-----+------------------+-----------+---------+-----------+------------+

Checking for those values to be non-nil shouldn't be too hard.

[alksmt]

Is it supposed that a record could be created in future? If so, we are having a second issue: when a record in future is created, and another record is on track now, it would potentially collide with each other and we will end up with overlaping records.

As a solution, records in future could be rejected or rejected only if another record is still on track, but with the second way potential overlapping is still possible.

Probably issue with records in future is not so related to this one and should be created separately.

[dominikbraun]

Is it supposed that a record could be created in future? If so, we are having a second issue: when a record in future is created, and another record is on track now, it would potentially collide with each other and we will end up with overlaping records.

As a solution, records in future could be rejected or rejected only if another record is still on track, but with the second way potential overlapping is still possible.

Probably issue with records in future is not so related to this one and should be created separately.

I think it would make sense to reject records in the future so that records can only be created retroactively. You may open an issue for this topic if you want to, otherwise I can do it 😄

[alksmt]

Will create one 👍

[alksmt]

Created PR for this bug, with some simpification for collides check, going to also handle future records rejection in new issue.