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 crash-report capabilities #574

Open
wants to merge 6 commits into
base: devel
from
Open

add crash-report capabilities #574

wants to merge 6 commits into from

Conversation

@umbynos
Copy link
Collaborator

@umbynos umbynos commented Nov 30, 2020

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • Tests for the changes have been added (for bug fixes / features)
  • What kind of change does this PR introduce?

feature

  • What is the current behavior?

Sometimes the agent crashes without apparent reason

  • What is the new behavior?

A file containing a crash-report is generated in that same folder containing the executable (e.g. in Linux it's in ~/ArduinoCreateAgent-<version>/) when the agent crashes. It should allow users to better report issues, and developers to better understand and fix bugs.
The file is named crashreport_yyyyMMddHHmmss.txt and should contain something like:

stacktrace from panic: 
goroutine 1 [running, locked to thread]:
runtime/debug.Stack(0x0, 0x0, 0x0)
	/home/linuxbrew/.linuxbrew/Cellar/go/1.15.3/libexec/src/runtime/debug/stack.go:24 +0xa5
main.panicToFile()
	/home/umberto/Nextcloud/8tb/Lavoro/arduino-create-agent/main.go:165 +0x22d
panic(0xa09400, 0xb0ebb0)
	/home/linuxbrew/.linuxbrew/Cellar/go/1.15.3/libexec/src/runtime/panic.go:975 +0x4c6
main.main()
	/home/umberto/Nextcloud/8tb/Lavoro/arduino-create-agent/main.go:118 +0x98
  • Does this PR introduce a breaking change?

no

  • Other information:
@umbynos umbynos changed the title add first implementation of a function generating a crash-report add crash-report capabilities Nov 30, 2020
@umbynos umbynos self-assigned this Nov 30, 2020
main.go Outdated
fileName := "crashreport_" + time.Now().Format("20060102150405") + ".txt"
currDir, err := osext.ExecutableFolder()
if err != nil {
panic(err)

This comment has been minimized.

@umbynos

umbynos Nov 30, 2020
Author Collaborator

I don't know if it's correct to panic inside a defer function. Maybe it's better to log the error. Returning the error is not possible.

@@ -111,6 +111,9 @@ func main() {
os.Exit(0)
}

// function used to save panic to a file (WIP)
defer panicToFile()

This comment has been minimized.

@umbynos

umbynos Nov 30, 2020
Author Collaborator

To try the feature I added
panic("test") after the defer and the file generated contains this stacktrace:

stacktrace from panic: 
goroutine 1 [running, locked to thread]:
runtime/debug.Stack(0x0, 0x0, 0x0)
	/home/linuxbrew/.linuxbrew/Cellar/go/1.15.3/libexec/src/runtime/debug/stack.go:24 +0xa5
main.panicToFile()
	/home/umberto/Nextcloud/8tb/Lavoro/arduino-create-agent/main.go:165 +0x22d
panic(0xa09400, 0xb0ebb0)
	/home/linuxbrew/.linuxbrew/Cellar/go/1.15.3/libexec/src/runtime/panic.go:975 +0x4c6
main.main()
	/home/umberto/Nextcloud/8tb/Lavoro/arduino-create-agent/main.go:117 +0x98

And this is stderr:

panic: test [recovered]
	panic: test

goroutine 1 [running, locked to thread]:
main.panicToFile()
	/home/umberto/Nextcloud/8tb/Lavoro/arduino-create-agent/main.go:170 +0x49b
panic(0xa09400, 0xb0ebb0)
	/home/linuxbrew/.linuxbrew/Cellar/go/1.15.3/libexec/src/runtime/panic.go:975 +0x4c6
main.main()
	/home/umberto/Nextcloud/8tb/Lavoro/arduino-create-agent/main.go:117 +0x98
@umbynos
Copy link
Collaborator Author

@umbynos umbynos commented Nov 30, 2020

Problems still to be solved:

  1. if the panic happens in the loop() function the panic does not get stopped because loop() is run in a different goroutine.

The process continues up the stack until all functions in the current goroutine have returned, at which point the program crashes
https://blog.golang.org/defer-panic-and-recover

  1. Handling of the old crash-reports still missing (which is the best way to do it?)
@umbynos
Copy link
Collaborator Author

@umbynos umbynos commented Dec 1, 2020

I've changed the approach: now everything happening on stderr is redirected to the crash-report saved locally. I've followed this issue There's still work to do regarding stdout and stderr. Now every output is redirected to the file (maybe because stdout and stderr are treated the same way(?))
Another possible problem concerns crash-report handling (how not to spam too many files)

@umbynos
Copy link
Collaborator Author

@umbynos umbynos commented Dec 1, 2020

Example of crash-report generated on windows.
crashreport_20201201165111.txt

@zmoog
Copy link
Collaborator

@zmoog zmoog commented Dec 2, 2020

Useful suggestions/enhancements:

  • Add a command in the systray (or a flag in the confi.ini) to clean/enable the logs
  • replace .txt -> .log extension for the log file
@umbynos umbynos force-pushed the umbynos/add_crashreport branch from 78dd953 to ac0f713 Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.