mattn / anko

Scriptable interpreter written in golang
http://play-anko.appspot.com/
MIT License
1.45k stars 120 forks source link

add option for panic capturing in Exec/Run #312

Closed complyue closed 4 years ago

complyue commented 5 years ago

In running scripts, Anko will capture panics unless run in debug mode.

My case is that I wrote a framework, with some library functions meant to be called by application code, the applications are supposed to be written in part Go , part Anko script. And the framework accepts scripts from elsewhere, and run them with an Anko env. While the applications may be mis-using the library functions sometimes, in which case a library function will panic with error description. And since the scripts come from many where and executes with great concurrency, just the error description is far from adequate to locate which application and which part of it caused the error, full stack trace is much desired for trouble shooting few bugs among massive concurrent tasks running smoothly in a production system, where I can't put Anko in debug mode.

What I'm suggesting in this PR is that an Unsafe version of ExecuteContext and RunContext be added, with an option argument capturePanic bool, to specify whether the caller intends to capture panics from the script code by itself. And existing ExecuteContext and RunContext stay the same behavior wrt the ${ANKO_DEBUG} environment variable.

Another option may be to coerce whatever captured into error type for runInfo.err in more sophisticated ways, I would do that by a custom errors module in each of my Go projects, like:

runInfo.err = errors.RichError(recoverResult)
package errors

import (
    "fmt"

    "github.com/pkg/errors"
)

var (
    New    = errors.New
    Errorf = errors.Errorf
    Wrap   = errors.Wrap
    Wrapf  = errors.Wrapf
)

// github.com/pkg/errors can be formatted with rich information, including stacktrace, see:
//  https://godoc.org/github.com/pkg/errors#hdr-Formatted_printing_of_errors
type richError interface {
    error
    fmt.Formatter
}

// wrap as necessary an object with rich (stacktrace esp.) information.
func RichError(err interface{}) error {
    if err == nil {
        return nil
    }
    switch err := err.(type) {
    case richError:
        return err
    case error:
        return errors.Wrap(err, err.Error()).(richError)
    default:
        return errors.New(fmt.Sprintf("%s", err)).(richError)
    }
}

But I think that's only a temporary solution before the std errors module provides richer info ultimately, so am not willing to persuade others to do alike.

complyue commented 5 years ago

oops, Travis failed, but my local tests all passed. help?

MichaelS11 commented 5 years ago

I am still not sure how exactly how error handling should be improved but I feel like adding a boolin options to main calling functions is not the best path to take.

I was generally thinking there might be a way to improve the VM error but have not taking the time to think about it much. https://github.com/mattn/anko/blob/master/vm/vm.go#L64-L66

MichaelS11 commented 5 years ago

Are there any examples of other programs using StackTrace?

https://godoc.org/github.com/pkg/errors#StackTrace

complyue commented 5 years ago

I am still not sure how exactly how error handling should be improved but I feel like adding a boolin options to main calling functions is not the best path to take.

Yes, I agree. For better API design there are better ways, I looked at https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis and personally feel config struct described there may be enough, or functional options may be the way to go. But that's a decision for Anko authors to make, I don't even know if there will be one another option to be added, and without more and more options to add, it's no hurry to make any decision.

I was generally thinking there might be a way to improve the VM error but have not taking the time to think about it much. https://github.com/mattn/anko/blob/master/vm/vm.go#L64-L66

I'm not sure if Anko has already supported recursive function definition, if a func defined by script can not call itself recursively, and mutual recursion neither supported, I do think a line number and script file name is enough. But if recursion is well supported and meant to be used intensively, I suppose script authors will be needing stacktraces on errors to help debug script problems.

And I suggest goroutine stack trace and Anko script error (with stack frames or not) better be considered separate things, in either case. The former is mainly to track down problems in scripted pieces written in Go, not the scripting part.

Are there any examples of other programs using StackTrace?

I don't know specific open code for that, but I think few programs will be parsing stack frames then do sth, most would be implicitly benefited from the full stacktrace printed out on panic or SIGQUIT.

MichaelS11 commented 5 years ago

Also can look at:

https://godoc.org/github.com/pkg/errors#WithStack

MichaelS11 commented 4 years ago

@complyue No longer going to do this? Close this?

complyue commented 4 years ago

sure, let's close this for now.