Skip to content

Commit

Permalink
Simplify error handling for report
Browse files Browse the repository at this point in the history
Aborts are assumed to be ok. The api is responsible for generating a
message together with the abort if it wants to communicate an issue.

All non-abort errors returned from mutate are internal errors.

Fixes #1082
  • Loading branch information
dsrbecky committed Sep 8, 2017
1 parent 7ca445a commit 13179cb
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 23 deletions.
2 changes: 1 addition & 1 deletion gapis/api/cmd_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ func Abort(msg string) *ErrCmdAborted {
// IsErrCmdAborted returns true if the cause of the error err was due to an
// abort() in the command.
func IsErrCmdAborted(err error) bool {
_, ok := errors.Cause(err).(ErrCmdAborted)
_, ok := errors.Cause(err).(*ErrCmdAborted)
return ok
}
7 changes: 0 additions & 7 deletions gapis/api/gles/find_issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,6 @@ func (t *findIssues) Transform(ctx context.Context, id api.CmdID, cmd api.Cmd, o
cb := CommandBuilder{Thread: cmd.Thread()}
t.lastGlError = GLenum_GL_NO_ERROR
mutateErr := cmd.Mutate(ctx, id, t.state, nil /* no builder */)
if mutateErr != nil {
if api.IsErrCmdAborted(mutateErr) && t.lastGlError != GLenum_GL_NO_ERROR {
// GL errors have already been reported - so do not log it again.
} else {
t.onIssue(cmd, id, service.Severity_ErrorLevel, mutateErr)
}
}

mutatorsGlError := t.lastGlError
if e := FindErrorState(cmd.Extras()); e != nil {
Expand Down
2 changes: 1 addition & 1 deletion gapis/api/gles/gles.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (c *State) preMutate(ctx context.Context, s *api.State, cmd api.Cmd) error
if f := s.NewMessage; f != nil {
f(log.Error, messages.ErrNoContextBound(cmd.Thread()))
}
return api.ErrCmdAborted{Reason: "No context bound"}
return &api.ErrCmdAborted{Reason: "No context bound"}
}
return nil
}
Expand Down
4 changes: 4 additions & 0 deletions gapis/messages/en-us.stb.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ Internal error in trace assert: {{reason}}

{{error}}

# ERR_INTERNAL_ERROR

Internal error: {{error}}

# ERR_REPLAY_DRIVER

Error during replay: {{replayError}}
Expand Down
18 changes: 4 additions & 14 deletions gapis/resolve/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package resolve

import (
"context"
"fmt"

"github.com/google/gapid/core/log"
"github.com/google/gapid/gapis/api"
Expand Down Expand Up @@ -70,13 +69,9 @@ func (r *ReportResolvable) Resolve(ctx context.Context) (interface{}, error) {

builder := service.NewReportBuilder()

var lastError interface{}
var currentAtom uint64
items := []*service.ReportItemRaw{}
state := c.NewState()
state.OnError = func(err interface{}) {
lastError = err
}
state.NewMessage = func(s log.Severity, m *stringtable.Msg) uint32 {
items = append(items, r.newReportItem(s, currentAtom, m))
return uint32(len(items) - 1)
Expand Down Expand Up @@ -122,22 +117,17 @@ func (r *ReportResolvable) Resolve(ctx context.Context) (interface{}, error) {
// Gather report items from the state mutator, and collect together all the
// APIs in use.
api.ForeachCmd(ctx, c.Commands, func(ctx context.Context, id api.CmdID, cmd api.Cmd) error {
items, lastError, currentAtom = items[:0], nil, uint64(id)
items, currentAtom = items[:0], uint64(id)

if as := cmd.Extras().Aborted(); as != nil && as.IsAssert {
items = append(items, r.newReportItem(log.Fatal, uint64(id),
messages.ErrTraceAssert(as.Reason)))
}

err := cmd.Mutate(ctx, id, state, nil /* no builder, just mutate */)

if len(items) == 0 {
if err != nil && !api.IsErrCmdAborted(err) {
items = append(items, r.newReportItem(log.Error, uint64(id),
messages.ErrMessage(err)))
} else if lastError != nil {
if err := cmd.Mutate(ctx, id, state, nil /* no builder, just mutate */); err != nil {
if !api.IsErrCmdAborted(err) {
items = append(items, r.newReportItem(log.Error, uint64(id),
messages.ErrMessage(fmt.Sprintf("%v", lastError))))
messages.ErrInternalError(err.Error())))
}
}

Expand Down

0 comments on commit 13179cb

Please sign in to comment.