Skip to content

[WIP] Errors add context#492

Open
dpordomingo wants to merge 3 commits intosrc-d:masterfrom
dpordomingo:errors-add-context
Open

[WIP] Errors add context#492
dpordomingo wants to merge 3 commits intosrc-d:masterfrom
dpordomingo:errors-add-context

Conversation

@dpordomingo
Copy link
Copy Markdown
Contributor

caused by #488

This PR adds a comments field to the logger, so when error is logged, it will appear the offending comments.

ERROR event processing failed
app=lookout
event-id=b44c158d09bebb40b85ce23763dd9ab179926019
event-type=*pb.ReviewEvent
github.pr=63
head=refs/pull/63/head
repo=https://github.com/dpordomingo/testing-repo.git
error=posting analysis failed: github api error: review could not be pushed:
    POST https://api.github.com/repos/dpordomingo/testing-repo/pulls/63/reviews
    422 Unprocessable Entity
    [{Resource:Review
      Message:Path is invalid
    }]

comments={commit:4718b67c9a53d95cd53c6da8d25e5e990a1752a1 
    body:
    event:COMMENT
    comments:[
        {path:README.md pos:1 body:language: Markdown}
        {path:golang.go pos:1 body:language: Go}
        ...
    ]}

benefits

  • comments that source{d} Lookout tried to push

these ☝️ new details would have been really useful when debugging #486

cons

Arised by @carlosms:

We are doing something wrong, WithLogFields is supposed to return a copy of ctx with the new values, but leaving the original ctx intact

#488 (comment)

The code is working but it shouldn't 😅 The original ctx log fields should not be modified... #491

#488 (comment)

Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
@dpordomingo dpordomingo self-assigned this Jan 25, 2019
@dpordomingo dpordomingo mentioned this pull request Jan 25, 2019
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant