Skip to content

Commit

Permalink
codec: clean up error handling - merge decodeError and encodeError in…
Browse files Browse the repository at this point in the history
…to codecError, and clean up Must(En|De)code to check for handle
  • Loading branch information
ugorji committed Jan 10, 2021
1 parent ec0371e commit a57a70f
Show file tree
Hide file tree
Showing 13 changed files with 111 additions and 112 deletions.
1 change: 1 addition & 0 deletions codec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ type DecodeOptions struct{ ... }
type Decoder struct{ ... }
func NewDecoder(r io.Reader, h Handle) *Decoder
func NewDecoderBytes(in []byte, h Handle) *Decoder
func NewDecoderString(s string, h Handle) *Decoder
type EncodeOptions struct{ ... }
type Encoder struct{ ... }
func NewEncoder(w io.Writer, h Handle) *Encoder
Expand Down
12 changes: 7 additions & 5 deletions codec/bench/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ func benchPreInit() {
// }

// use zerocopy for the benchmarks, for best performance
testJsonH.ZeroCopy = true
testCborH.ZeroCopy = true
testMsgpackH.ZeroCopy = true
testSimpleH.ZeroCopy = true
testBincH.ZeroCopy = true
const zeroCopyVal = true

testJsonH.ZeroCopy = zeroCopyVal
testCborH.ZeroCopy = zeroCopyVal
testMsgpackH.ZeroCopy = zeroCopyVal
testSimpleH.ZeroCopy = zeroCopyVal
testBincH.ZeroCopy = zeroCopyVal
}

func benchReinit() {
Expand Down
6 changes: 3 additions & 3 deletions codec/bench/z_all_x_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func benchmarkXGroup(t *testing.B) {
t.Run("Benchmark__Fxcbor_____Decode", Benchmark__Fxcbor_____Decode)
t.Run("Benchmark__Bson_______Decode", Benchmark__Bson_______Decode)
t.Run("Benchmark__Mgobson____Decode", Benchmark__Mgobson____Decode)
// t.Run("Benchmark__VMsgpack___Decode", Benchmark__VMsgpack___Decode)
t.Run("Benchmark__VMsgpack___Decode", Benchmark__VMsgpack___Decode)
// t.Run("Benchmark__Gcbor______Decode", Benchmark__Gcbor______Decode)
// t.Run("Benchmark__Xdr________Decode", Benchmark__Xdr________Decode)
// t.Run("Benchmark__Sereal_____Decode", Benchmark__Sereal_____Decode)
Expand Down Expand Up @@ -72,14 +72,14 @@ func benchmarkCodecXGroup(t *testing.B) {
t.Run("Benchmark__JsonIter___Decode", Benchmark__JsonIter___Decode)
t.Run("Benchmark__Bson_______Decode", Benchmark__Bson_______Decode)
t.Run("Benchmark__Mgobson____Decode", Benchmark__Mgobson____Decode)
// t.Run("Benchmark__VMsgpack___Decode", Benchmark__VMsgpack___Decode)
t.Run("Benchmark__VMsgpack___Decode", Benchmark__VMsgpack___Decode)
t.Run("Benchmark__Fxcbor_____Decode", Benchmark__Fxcbor_____Decode)
// t.Run("Benchmark__Gcbor______Decode", Benchmark__Gcbor______Decode)
// t.Run("Benchmark__Xdr________Decode", Benchmark__Xdr________Decode)
// t.Run("Benchmark__Sereal_____Decode", Benchmark__Sereal_____Decode)
}

var benchmarkXSkipMsg = `>>>> Skipping - these cannot (en|de)code TestStruc - encode (gcbor, xdr, xml), decode (gcbor, vmsgpack, xdr, sereal, xml)`
var benchmarkXSkipMsg = `>>>> Skipping - these cannot (en|de)code TestStruc - encode (xml, gcbor, xdr), decode (xml, gcbor, xdr, sereal)`

func BenchmarkXSuite(t *testing.B) {
println(benchmarkXSkipMsg)
Expand Down
2 changes: 1 addition & 1 deletion codec/binc.go
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ func (d *bincDecDriver) DecodeBytes(bs []byte) (bsOut []byte) {
d.d.errorf("bytes - %s %x-%x/%s", msgBadDesc, d.vd, d.vs, bincdesc(d.vd, d.vs))
}
d.bdRead = false
if d.d.bytes && d.h.ZeroCopy {
if d.d.zerocopy() {
d.d.decByteState = decByteStateZerocopy
return d.d.decRd.rb.readx(uint(clen))
}
Expand Down
2 changes: 1 addition & 1 deletion codec/cbor.go
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ func (d *cborDecDriver) DecodeBytes(bs []byte) (bsOut []byte) {
}
clen := d.decLen()
d.bdRead = false
if d.d.bytes && d.h.ZeroCopy {
if d.d.zerocopy() {
d.d.decByteState = decByteStateZerocopy
return d.d.decRd.rb.readx(uint(clen))
}
Expand Down
24 changes: 15 additions & 9 deletions codec/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1507,11 +1507,20 @@ func doTestMapEncodeForCanonical(t *testing.T, h Handle) {
if ch, ok := h.(*CborHandle); ok {
cborIndef = ch.IndefiniteLength
}

// if ch, ok := h.(*BincHandle); ok && ch.AsSymbols != 2 {
// defer func(u uint8) { ch.AsSymbols = u }(ch.AsSymbols)
// ch.AsSymbols = 2
// }

bh := testBasicHandle(h)
if !bh.Canonical {
bh.Canonical = true
defer func() { bh.Canonical = false }()
}

defer func(c, si bool) {
bh.Canonical = c
bh.SignedInteger = si
}(bh.Canonical, bh.SignedInteger)
bh.Canonical = true
// bh.SignedInteger = true

e1 := NewEncoderBytes(&b1, h)
e1.MustEncode(v1)
Expand Down Expand Up @@ -1642,7 +1651,7 @@ func doTestErrWriter(t *testing.T, h Handle) {
enc := NewEncoder(w, h)
for i := 0; i < 4; i++ {
err := enc.Encode("ugorji")
if ev, ok := err.(encodeError); ok {
if ev, ok := err.(*codecError); ok {
err = ev.Cause()
}
if err != testErrWriterErr {
Expand Down Expand Up @@ -3007,11 +3016,8 @@ func doTestMaxDepth(t *testing.T, h Handle) {
var v2 interface{}
err = testUnmarshal(&v2, b1, h)
}
if err1, ok := err.(decodeError); ok {
err = err1.codecError
}
var err0 interface{} = err
if err1, ok := err.(codecError); ok {
if err1, ok := err.(*codecError); ok {
err0 = err1.err
}
if err0 != v.E {
Expand Down
70 changes: 25 additions & 45 deletions codec/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package codec
import (
"encoding"
"errors"
"fmt"
"io"
"reflect"
"strconv"
Expand Down Expand Up @@ -156,15 +155,6 @@ type decDriverContainerTracker interface {
ReadMapElemValue()
}

type decodeError struct {
codecError
pos int
}

func (d decodeError) Error() string {
return fmt.Sprintf("%s decode error [pos %d]: %v", d.name, d.pos, d.err)
}

type decDriverNoopContainerReader struct{}

func (x decDriverNoopContainerReader) ReadArrayStart() (v int) { return }
Expand Down Expand Up @@ -1015,18 +1005,6 @@ func (d *Decoder) kMap(f *codecFnInfo, rv reflect.Value) {

var callFnRvk bool

// fnRvk is not inlineable, as reflect.Value.Set has high inline cost
// so we just manually inline in the 2 places below.
// fnRvk := func(s string) {
// if callFnRvk {
// if ktypeIsString {
// rvSetString(rvk, s)
// } else { // ktypeIsIntf
// rvk.Set(rv4i(s))
// }
// }
// }

fnRvk2 := func() (s string) {
callFnRvk = false
if len(kstr2bs) < 2 {
Expand All @@ -1035,7 +1013,8 @@ func (d *Decoder) kMap(f *codecFnInfo, rv reflect.Value) {
if safeMode {
return d.string(kstr2bs)
}
if !safeMode && d.decByteState == decByteStateZerocopy && d.h.ZeroCopy {
// unsafe mode below
if d.decByteState == decByteStateZerocopy && d.h.ZeroCopy {
return stringView(kstr2bs)
}
callFnRvk = true
Expand Down Expand Up @@ -1225,6 +1204,10 @@ func NewDecoderBytes(in []byte, h Handle) *Decoder {
return d
}

func NewDecoderString(s string, h Handle) *Decoder {
return NewDecoderBytes(bytesView(s), h)
}

func (d *Decoder) r() *decRd {
return &d.decRd
}
Expand Down Expand Up @@ -1270,7 +1253,7 @@ func (d *Decoder) resetCommon() {
// clearing all state from last run(s).
func (d *Decoder) Reset(r io.Reader) {
if r == nil {
return
r = &eofReader
}
d.bytes = false
if d.h.ReaderBufferSize > 0 {
Expand All @@ -1295,7 +1278,7 @@ func (d *Decoder) Reset(r io.Reader) {
// clearing all state from last run(s).
func (d *Decoder) ResetBytes(in []byte) {
if in == nil {
return
in = []byte{}
}
d.bufio = false
d.bytes = true
Expand All @@ -1304,6 +1287,10 @@ func (d *Decoder) ResetBytes(in []byte) {
d.resetCommon()
}

func (d *Decoder) ResetString(s string) {
d.ResetBytes(bytesView(s))
}

func (d *Decoder) naked() *fauxUnion {
return &d.n
}
Expand Down Expand Up @@ -1373,32 +1360,27 @@ func (d *Decoder) Decode(v interface{}) (err error) {
// tried to use closure, as runtime optimizes defer with no params.
// This seemed to be causing weird issues (like circular reference found, unexpected panic, etc).
// Also, see https://github.com/golang/go/issues/14939#issuecomment-417836139
if d.err != nil {
return d.err
}
defer func() {
if x := recover(); x != nil {
panicValToErr(d, x, &err)
d.err = err
panicValToErr(d, x, &d.err)
err = d.err
}
}()

d.mustDecode(v)
d.MustDecode(v)
return
}

// MustDecode is like Decode, but panics if unable to Decode.
// This provides insight to the code location that triggered the error.
//
// Note: This provides insight to the code location that triggered the error.
func (d *Decoder) MustDecode(v interface{}) {
halt.onerror(d.err)
d.mustDecode(v)
}
if d.hh == nil {
halt.onerror(errNoFormatHandle)
}

// MustDecode is like Decode, but panics if unable to Decode.
// This provides insight to the code location that triggered the error.
func (d *Decoder) mustDecode(v interface{}) {
// Top-level: v is a pointer and not nil.

d.calls++
d.decode(v)
d.calls--
Expand Down Expand Up @@ -1679,12 +1661,6 @@ func (d *Decoder) depthDecr() {
// This should mostly be used for map keys, where the key type is string.
// This is because keys of a map/struct are typically reused across many objects.
func (d *Decoder) string(v []byte) (s string) {
// if !d.h.InternString || d.c != containerMapKey || len(v) < 2 || len(v) > internMaxStrLen {
// return string(v)
// }
// if d.is == nil {
// d.is.init()
// }
if d.is == nil || d.c != containerMapKey || len(v) < 2 || len(v) > internMaxStrLen {
return string(v)
}
Expand All @@ -1698,6 +1674,10 @@ func (d *Decoder) stringZC(v []byte) (s string) {
return d.string(v)
}

func (d *Decoder) zerocopy() bool {
return d.bytes && d.h.ZeroCopy
}

// decodeBytesInto is a convenience delegate function to decDriver.DecodeBytes.
// It ensures that `in` is not a nil byte, before calling decDriver.DecodeBytes,
// as decDriver.DecodeBytes treats a nil as a hint to use its internal scratch buffer.
Expand All @@ -1715,7 +1695,7 @@ func (d *Decoder) rawBytes() (v []byte) {
}

func (d *Decoder) wrapErr(v error, err *error) {
*err = decodeError{codecError: codecError{name: d.hh.Name(), err: v}, pos: d.NumBytesRead()}
*err = wrapCodecErr(v, d.hh.Name(), d.NumBytesRead(), false)
}

// NumBytesRead returns the number of bytes read
Expand Down
30 changes: 9 additions & 21 deletions codec/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package codec
import (
"encoding"
"errors"
"fmt"
"io"
"reflect"
"sort"
Expand Down Expand Up @@ -50,14 +49,6 @@ type encDriverContainerTracker interface {
WriteMapElemValue()
}

type encodeError struct {
codecError
}

func (e encodeError) Error() string {
return fmt.Sprintf("%s encode error: %v", e.name, e.err)
}

type encDriverNoopContainerWriter struct{}

func (encDriverNoopContainerWriter) WriteArrayStart(length int) {}
Expand Down Expand Up @@ -978,31 +969,28 @@ func (e *Encoder) Encode(v interface{}) (err error) {
// tried to use closure, as runtime optimizes defer with no params.
// This seemed to be causing weird issues (like circular reference found, unexpected panic, etc).
// Also, see https://github.com/golang/go/issues/14939#issuecomment-417836139

if e.err != nil {
return e.err
}
defer func() {
// if error occurred during encoding, return that error;
// else if error occurred on end'ing (i.e. during flush), return that error.
if x := recover(); x != nil {
panicValToErr(e, x, &err)
e.err = err
panicValToErr(e, x, &e.err)
err = e.err
}
}()

e.mustEncode(v)
e.MustEncode(v)
return
}

// MustEncode is like Encode, but panics if unable to Encode.
// This provides insight to the code location that triggered the error.
//
// Note: This provides insight to the code location that triggered the error.
func (e *Encoder) MustEncode(v interface{}) {
halt.onerror(e.err)
e.mustEncode(v)
}
if e.hh == nil {
halt.onerror(errNoFormatHandle)
}

func (e *Encoder) mustEncode(v interface{}) {
e.calls++
e.encode(v)
e.calls--
Expand Down Expand Up @@ -1242,7 +1230,7 @@ func (e *Encoder) rawBytes(vv Raw) {
}

func (e *Encoder) wrapErr(v error, err *error) {
*err = encodeError{codecError{name: e.hh.Name(), err: v}}
*err = wrapCodecErr(v, e.hh.Name(), 0, true)
}

// ---- container tracker methods
Expand Down
Loading

0 comments on commit a57a70f

Please sign in to comment.