Skip to content

Commit

Permalink
Improve errors for logfile import
Browse files Browse the repository at this point in the history 
Before it would just display the API error, which kinda sucked. Now it
displays the line number + the offending line.
  • Loading branch information
@arp242
arp242 committed Mar 29, 2021
1 parent f880198 commit 3958a44
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 20 deletions.
36 changes: 21 additions & 15 deletions cmd/goatcounter/import.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,9 @@ func importLog(
return err
}

//zlog.Module("import").Debug(line)

hit := handlers.APICountRequestHit{
Line: line.Line(),
LineNo: line.LineNo(),
Path: line.Path(),
Ref: line.Referrer(),
Query: line.Query(),
Expand Down Expand Up @@ -388,27 +388,18 @@ func importLog(
hits <- hit
if len(hits) >= cap(hits) {
t.Reset(d)
if persistLog(hits, url, key, silent, follow) {
break
}
persistLog(hits, url, key, silent, follow)
}
}
return nil
}

// TODO: also add titles in the background.
// Ehm, probably best to let memstore do its job and insert path
// rows, and then run goroutine in background to update the lot?
//
// Maybe just select where title = '' and then try to update those
// one-by-one.

// Send everything off if we have 100 entries or if 10 seconds expired,
// whichever happens first.
func persistLog(hits <-chan handlers.APICountRequestHit, url, key string, silent, follow bool) bool {
func persistLog(hits <-chan handlers.APICountRequestHit, url, key string, silent, follow bool) {
l := len(hits)
if l == 0 {
return false
return
}
collect := make([]handlers.APICountRequestHit, l)
for i := 0; i < l; i++ {
Expand All @@ -419,7 +410,7 @@ func persistLog(hits <-chan handlers.APICountRequestHit, url, key string, silent
if err != nil {
zlog.Error(err)
}
return false
return
}

var (
Expand Down Expand Up @@ -448,6 +439,21 @@ func importSend(url, key string, silent, follow bool, hits []handlers.APICountRe

if resp.StatusCode != 202 {
b, _ := io.ReadAll(resp.Body)
var gcErr struct {
Errors map[int]string `json:"errors"`
}
err := json.Unmarshal(b, &gcErr)
if err == nil {
for i, e := range gcErr.Errors {
zlog.Fields(zlog.F{
"lineno": hits[i].LineNo,
"line": strings.TrimRight(hits[i].Line, "\r\n"),
"error": strings.TrimSpace(e),
}).Errorf("error processing line %d", hits[i].LineNo)
}
return nil
}

return fmt.Errorf("%s: %s: %s", url, resp.Status, b)
}

Expand Down
5 changes: 5 additions & 0 deletions handlers/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,11 @@ type APICountRequestHit struct {

// {omitdoc}
Host string `json:"-"`

// {omitdoc} Line when importing, for displaying errors.
Line string `json:"-"`
// {omitdoc} Line when importing, for displaying errors.
LineNo uint64 `json:"-"`
}

func (h APICountRequestHit) String() string {
Expand Down
20 changes: 15 additions & 5 deletions logscan/logscan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,10 @@ func getFormat(format, date, time, datetime string) (string, string, string, str
}

type Scanner struct {
read chan follow.Data
re *regexp.Regexp
names []string
read chan follow.Data
re *regexp.Regexp
names []string
lineno uint64

date, time, datetime string

Expand Down Expand Up @@ -281,7 +282,7 @@ func (s Scanner) DateFormats() (date, time, datetime string) {
}

// Line processes a single line.
func (s Scanner) Line(ctx context.Context) (Line, error) {
func (s *Scanner) Line(ctx context.Context) (Line, error) {
start:
var line string
select {
Expand All @@ -292,9 +293,12 @@ start:
return nil, r.Err
}
line = r.String()
s.lineno++
}

parsed := make(Line, len(s.names))
parsed := make(Line, len(s.names)+2)
parsed["_line"] = line
parsed["_lineno"] = strconv.FormatUint(s.lineno, 10)
for _, sub := range s.re.FindAllStringSubmatchIndex(line, -1) {
for i := 2; i < len(sub); i += 2 {
v := line[sub[i]:sub[i+1]]
Expand Down Expand Up @@ -351,9 +355,15 @@ func toI64(s string) int64 {
n, _ := strconv.ParseInt(s, 10, 64)
return n
}
func toUi64(s string) uint64 {
n, _ := strconv.ParseUint(s, 10, 64)
return n
}

type Line map[string]string

func (l Line) Line() string { return l["_line"] }
func (l Line) LineNo() uint64 { return toUi64(l["_lineno"]) }
func (l Line) Host() string { return l["host"] }
func (l Line) RemoteAddr() string { return l["remote_addr"] }
func (l Line) XForwardedFor() string { return l["xff"] }
Expand Down
13 changes: 13 additions & 0 deletions logscan/logscan_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func TestNew(t *testing.T) {
}
want := []Line{
{
"_lineno": "1",
"datetime": "10/Oct/2000:13:55:36 -0700",
"host": "example.com",
"http": "HTTP/1.1",
Expand All @@ -49,6 +50,7 @@ func TestNew(t *testing.T) {
"user_agent": "Mozilla/5.0",
},
{
"_lineno": "2",
"datetime": "10/Oct/2000:13:55:36 -0700",
"host": "example.com",
"http": "HTTP/1.1",
Expand Down Expand Up @@ -100,6 +102,8 @@ func TestNew(t *testing.T) {
delete(w, "user_agent")
}

delete(data, "_line")

if !reflect.DeepEqual(data, w) {
t.Errorf("\nwant: %v\ngot: %v", w, data)
}
Expand Down Expand Up @@ -189,6 +193,7 @@ func TestNewFollow(t *testing.T) {

want := []Line{
{
"_lineno": "1",
"datetime": "10/Oct/2000:13:55:36 -0700",
"host": "example.com",
"http": "HTTP/1.1",
Expand All @@ -201,6 +206,7 @@ func TestNewFollow(t *testing.T) {
"user_agent": "Mozilla/5.0",
},
{
"_lineno": "2",
"datetime": "10/Oct/2001:13:55:36 -0700",
"host": "example.com",
"http": "HTTP/1.1",
Expand All @@ -213,6 +219,7 @@ func TestNewFollow(t *testing.T) {
"user_agent": "Mozilla/5.0",
},
{
"_lineno": "3",
"datetime": "10/Oct/2001:13:55:36 -0700",
"host": "example.com",
"http": "HTTP/1.1",
Expand All @@ -225,6 +232,7 @@ func TestNewFollow(t *testing.T) {
"user_agent": "Mozilla/5.0",
},
{
"_lineno": "4",
"datetime": "10/Oct/2001:13:55:36 -0700",
"host": "example.org",
"http": "HTTP/1.1",
Expand All @@ -237,6 +245,7 @@ func TestNewFollow(t *testing.T) {
"user_agent": "Mozilla/5.0",
},
{
"_lineno": "5",
"datetime": "10/Oct/2000:13:55:36 -0700",
"host": "example.com",
"http": "HTTP/1.1",
Expand All @@ -249,6 +258,7 @@ func TestNewFollow(t *testing.T) {
"user_agent": "Mozilla/5.0",
},
{
"_lineno": "6",
"datetime": "10/Oct/2001:13:55:36 -0700",
"host": "example.com",
"http": "HTTP/1.1",
Expand All @@ -261,6 +271,7 @@ func TestNewFollow(t *testing.T) {
"user_agent": "Mozilla/5.0",
},
{
"_lineno": "7",
"datetime": "10/Oct/2001:13:55:36 -0700",
"host": "example.com",
"http": "HTTP/1.1",
Expand All @@ -273,6 +284,7 @@ func TestNewFollow(t *testing.T) {
"user_agent": "Mozilla/5.0",
},
{
"_lineno": "8",
"datetime": "10/Oct/2001:13:55:36 -0700",
"host": "example.org",
"http": "HTTP/1.1",
Expand All @@ -287,6 +299,7 @@ func TestNewFollow(t *testing.T) {
}

for i := range data {
delete(data[i], "_line")
if !reflect.DeepEqual(data[i], want[i]) {
t.Errorf("line %d\nwant: %#v\ngot: %#v", i, want[i], data[i])
}
Expand Down

0 comments on commit 3958a44

Please sign in to comment.