Skip to content

Commit 3c0b16b

Browse files
astanslerastansler
authored
feat: error handling for common crawler (APP-106) (#42)
* feat: common error handling for observers (APP-106) * chore: fix spelling * wip: hashing error processing
1 parent 133fa9f commit 3c0b16b

File tree

8 files changed

+86
-39
lines changed

8 files changed

+86
-39
lines changed

‎src/main/kotlin/app/Logger.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ object Logger {
4444
/**
4545
* Log error message with exception info.
4646
*/
47-
fun error(message: String, e: Exception) {
47+
fun error(message: String, e: Throwable) {
4848
if (LEVEL >= ERROR) {
4949
println("[e] $message: $e")
5050
}

‎src/main/kotlin/app/hashers/CommitHasher.kt‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,11 @@ class CommitHasher(private val localRepo: LocalRepo,
6161
}
6262

6363
// Hash added and missing server commits and send them to server.
64-
fun updateFromObservable(observable: Observable<Commit>) {
64+
fun updateFromObservable(observable: Observable<Commit>,
65+
onError: (Throwable) -> Unit) {
6566
val lastKnownCommit = serverRepo.commits.lastOrNull()
6667
val knownCommits = serverRepo.commits.toHashSet()
6768

68-
val throwables = mutableListOf<Throwable>()
69-
7069
observable
7170
.takeWhile { new -> // Hash until last known commit.
7271
new.rehash != lastKnownCommit?.rehash
@@ -94,9 +93,6 @@ class CommitHasher(private val localRepo: LocalRepo,
9493
.buffer(20, TimeUnit.SECONDS) // Group ready commits by time.
9594
.subscribe({ commitsBundle -> // OnNext.
9695
postCommitsToServer(commitsBundle) // Send ready commits.
97-
}, { e -> // OnError.
98-
Logger.error("Hashing error: " + e.message)
99-
throwables.add(e) // TODO(anatoly): Top-class handling errors.
100-
})
96+
}, onError)
10197
}
10298
}

‎src/main/kotlin/app/hashers/FactHasher.kt‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,11 @@ class FactHasher(private val localRepo: LocalRepo,
2929
}
3030
}
3131

32-
fun updateFromObservable(observable: Observable<Commit>) {
32+
fun updateFromObservable(observable: Observable<Commit>,
33+
onError: (Throwable) -> Unit) {
3334
val factsDayWeek = hashMapOf<Author, Array<Int>>()
3435
val factsDayTime = hashMapOf<Author, Array<Int>>()
3536

36-
val throwables = mutableListOf<Throwable>()
37-
3837
// TODO(anatoly): Filter hashing by email as in CommitHasher.
3938
observable
4039
.subscribe({ commit -> // OnNext.
@@ -51,9 +50,7 @@ class FactHasher(private val localRepo: LocalRepo,
5150
factDayTime[dateTime.hour] += 1
5251
factsDayWeek[author] = factDayWeek
5352
factsDayTime[author] = factDayTime
54-
}, { e -> // OnError.
55-
throwables.add(e) // TODO(anatoly): Top-class handling errors.
56-
}, { // OnComplete.
53+
}, onError, { // OnComplete.
5754
try {
5855
val facts = mutableListOf<Fact>()
5956
factsDayTime.map { (author, list) ->
@@ -76,7 +73,7 @@ class FactHasher(private val localRepo: LocalRepo,
7673
}
7774
postFactsToServer(facts)
7875
} catch (e: Throwable) {
79-
throwables.add(e)
76+
onError(e)
8077
}
8178
})
8279
}

‎src/main/kotlin/app/hashers/RepoHasher.kt‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ package app.hashers
66
import app.Logger
77
import app.api.Api
88
import app.config.Configurator
9-
import app.model.Author
109
import app.model.LocalRepo
1110
import app.model.Repo
11+
import app.utils.HashingException
1212
import app.utils.RepoHelper
1313
import org.apache.commons.codec.digest.DigestUtils
1414
import org.eclipse.jgit.api.Git
@@ -26,7 +26,9 @@ class RepoHasher(private val localRepo: LocalRepo, private val api: Api,
2626
if (!RepoHelper.isValidRepo(localRepo.path)) {
2727
throw IllegalArgumentException("Invalid repo $localRepo")
2828
}
29+
}
2930

31+
fun update() {
3032
println("Hashing $localRepo...")
3133
val git = loadGit(localRepo.path)
3234
try {
@@ -46,26 +48,44 @@ class RepoHasher(private val localRepo: LocalRepo, private val api: Api,
4648
// Get repo setup (commits, emails to hash) from server.
4749
getRepoFromServer()
4850

51+
// Common error handling for subscribers.
52+
// Exceptions can't be thrown out of reactive chain.
53+
val errors = mutableListOf<Throwable>()
54+
val onError: (Throwable) -> Unit = {
55+
e -> errors.add(e)
56+
Logger.error("Error while hashing:", e)
57+
}
58+
4959
// Hash by all plugins.
5060
val observable = CommitCrawler.getObservable(git, serverRepo)
5161
.publish()
5262
CommitHasher(localRepo, serverRepo, api, rehashes)
53-
.updateFromObservable(observable)
63+
.updateFromObservable(observable, onError)
5464
FactHasher(localRepo, serverRepo, api)
55-
.updateFromObservable(observable)
65+
.updateFromObservable(observable, onError)
5666
// Start and synchronously wait until all subscribers complete.
5767
observable.connect()
5868

5969
// TODO(anatoly): CodeLongevity hash from observable.
60-
CodeLongevity(localRepo, serverRepo, api, git).update()
70+
try {
71+
CodeLongevity(localRepo, serverRepo, api, git).update()
72+
}
73+
catch (e: Throwable) {
74+
onError(e)
75+
}
6176

6277
// Confirm hashing completion.
6378
postRepoToServer()
79+
80+
if (errors.isNotEmpty()) {
81+
throw HashingException(errors)
82+
}
83+
84+
println("Hashing $localRepo successfully finished.")
6485
}
6586
finally {
6687
closeGit(git)
6788
}
68-
println("Hashing $localRepo successfully finished.")
6989
}
7090

7191
private fun loadGit(path: String): Git {
@@ -104,29 +124,25 @@ class RepoHasher(private val localRepo: LocalRepo, private val api: Api,
104124
}
105125

106126
private fun fetchRehashesAndAuthors(git: Git):
107-
Pair<LinkedList<String>, HashMap<String, Author>> {
127+
Pair<LinkedList<String>, HashSet<String>> {
108128
val head: RevCommit = RevWalk(git.repository)
109129
.parseCommit(git.repository.resolve(RepoHelper.MASTER_BRANCH))
110130

111131
val revWalk = RevWalk(git.repository)
112132
revWalk.markStart(head)
113133

114134
val commitsRehashes = LinkedList<String>()
115-
val contributors = hashMapOf<String, Author>()
135+
val authors = hashSetOf<String>()
116136

117137
var commit: RevCommit? = revWalk.next()
118138
while (commit != null) {
119139
commitsRehashes.add(DigestUtils.sha256Hex(commit.name))
120-
if (!contributors.containsKey(commit.authorIdent.emailAddress)) {
121-
val author = Author(commit.authorIdent.name,
122-
commit.authorIdent.emailAddress)
123-
contributors.put(commit.authorIdent.emailAddress, author)
124-
}
140+
authors.add(commit.authorIdent.emailAddress)
125141
commit.disposeBody()
126142
commit = revWalk.next()
127143
}
128144
revWalk.dispose()
129145

130-
return Pair(commitsRehashes, contributors)
146+
return Pair(commitsRehashes, authors)
131147
}
132148
}

‎src/main/kotlin/app/ui/UpdateRepoState.kt‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import app.hashers.RepoHasher
77
import app.Logger
88
import app.api.Api
99
import app.config.Configurator
10+
import app.utils.HashingException
1011
import app.utils.RequestException
1112

1213
/**
@@ -20,9 +21,12 @@ class UpdateRepoState constructor(private val context: Context,
2021
println("Hashing your git repositories.")
2122
for (repo in configurator.getLocalRepos()) {
2223
try {
23-
RepoHasher(repo, api, configurator)
24-
} catch (e: RequestException) {
25-
Logger.error("Network error while hashing $repo", e)
24+
RepoHasher(repo, api, configurator).update()
25+
} catch (e: HashingException) {
26+
Logger.error("During hashing ${e.errors.size} errors occurred:")
27+
e.errors.forEach { error ->
28+
Logger.error("", error)
29+
}
2630
} catch (e: Exception) {
2731
Logger.error("Error while hashing $repo", e)
2832
}

‎src/main/kotlin/app/utils/HashingException.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Copyright 2017 Sourcerer Inc. All Rights Reserved.
2+
// Author: Anatoly Kislov ([email protected])
3+
4+
package app.utils
5+
6+
class HashingException(val errors: List<Throwable>) : Exception()

‎src/test/kotlin/test/tests/hashers/CommitHasherTest.kt‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,15 @@ class CommitHasherTest : Spek({
6363
given("repo with initial commit and no history") {
6464
repo.commits = listOf()
6565

66+
val errors = mutableListOf<Throwable>()
6667
val mockApi = MockApi(mockRepo = repo)
6768
val observable = CommitCrawler.getObservable(gitHasher, repo)
6869
CommitHasher(localRepo, repo, mockApi, repo.commits.map {it.rehash})
69-
.updateFromObservable(observable)
70+
.updateFromObservable(observable, { e -> errors.add(e) })
71+
72+
it ("has no errors") {
73+
assertEquals(0, errors.size)
74+
}
7075

7176
it("send added commits") {
7277
assertEquals(1, mockApi.receivedAddedCommits.size)
@@ -80,10 +85,15 @@ class CommitHasherTest : Spek({
8085
given("repo with initial commit") {
8186
repo.commits = listOf(getLastCommit(git))
8287

88+
val errors = mutableListOf<Throwable>()
8389
val mockApi = MockApi(mockRepo = repo)
8490
val observable = CommitCrawler.getObservable(gitHasher, repo)
8591
CommitHasher(localRepo, repo, mockApi, repo.commits.map {it.rehash})
86-
.updateFromObservable(observable)
92+
.updateFromObservable(observable, { e -> errors.add(e) })
93+
94+
it ("has no errors") {
95+
assertEquals(0, errors.size)
96+
}
8797

8898
it("doesn't send added commits") {
8999
assertEquals(0, mockApi.receivedAddedCommits.size)
@@ -97,13 +107,17 @@ class CommitHasherTest : Spek({
97107
given("happy path: added one commit") {
98108
repo.commits = listOf(getLastCommit(git))
99109

110+
val errors = mutableListOf<Throwable>()
100111
val mockApi = MockApi(mockRepo = repo)
101-
102112
val revCommit = git.commit().setMessage("Second commit.").call()
103113
val addedCommit = Commit(revCommit)
104114
val observable = CommitCrawler.getObservable(gitHasher, repo)
105115
CommitHasher(localRepo, repo, mockApi, repo.commits.map {it.rehash})
106-
.updateFromObservable(observable)
116+
.updateFromObservable(observable, { e -> errors.add(e) })
117+
118+
it ("has no errors") {
119+
assertEquals(0, errors.size)
120+
}
107121

108122
it("doesn't send deleted commits") {
109123
assertEquals(0, mockApi.receivedDeletedCommits.size)
@@ -121,6 +135,7 @@ class CommitHasherTest : Spek({
121135
given("happy path: added a few new commits") {
122136
repo.commits = listOf(getLastCommit(git))
123137

138+
val errors = mutableListOf<Throwable>()
124139
val mockApi = MockApi(mockRepo = repo)
125140

126141
val otherAuthorsNames = listOf("a", "b", "a")
@@ -139,7 +154,11 @@ class CommitHasherTest : Spek({
139154
}
140155
val observable = CommitCrawler.getObservable(gitHasher, repo)
141156
CommitHasher(localRepo, repo, mockApi, repo.commits.map {it.rehash})
142-
.updateFromObservable(observable)
157+
.updateFromObservable(observable, { e -> errors.add(e) })
158+
159+
it ("has no errors") {
160+
assertEquals(0, errors.size)
161+
}
143162

144163
it("posts five commits as added") {
145164
assertEquals(5, mockApi.receivedAddedCommits.size)
@@ -185,6 +204,7 @@ class CommitHasherTest : Spek({
185204
given("lost server") {
186205
repo.commits = listOf(getLastCommit(git))
187206

207+
val errors = mutableListOf<Throwable>()
188208
val mockApi = MockApi(mockRepo = repo)
189209

190210
// Add some commits.
@@ -201,7 +221,11 @@ class CommitHasherTest : Spek({
201221

202222
val observable = CommitCrawler.getObservable(gitHasher, repo)
203223
CommitHasher(localRepo, repo, mockApi, repo.commits.map {it.rehash})
204-
.updateFromObservable(observable)
224+
.updateFromObservable(observable, { e -> errors.add(e) })
225+
226+
it ("has no errors") {
227+
assertEquals(0, errors.size)
228+
}
205229

206230
it("adds posts one commit as added and received commit is lost one") {
207231
assertEquals(1, mockApi.receivedAddedCommits.size)

‎src/test/kotlin/test/tests/hashers/FactHasherTest.kt‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,12 @@ class FactHasherTest : Spek({
5555
date = createDate(year = 2017, month = 1, day = 1, // Sunday.
5656
hour = 13, minute = 0, seconds = 0))
5757

58+
val errors = mutableListOf<Throwable>()
5859
val observable = CommitCrawler.getObservable(testRepo.git, repo)
5960
FactHasher(localRepo, repo, mockApi)
60-
.updateFromObservable(observable)
61+
.updateFromObservable(observable, { e -> errors.add(e) })
6162

63+
assertEquals(0, errors.size)
6264
assertEquals(2, facts.size)
6365
assertTrue(facts.contains(Fact(repo, FactCodes.COMMITS_DAY_TIME, 13,
6466
1.0, author1)))
@@ -79,10 +81,12 @@ class FactHasherTest : Spek({
7981
date = createDate(year=2017, month = 1, day = 2, // Monday.
8082
hour = 13, minute = 0, seconds = 0))
8183

84+
val errors = mutableListOf<Throwable>()
8285
val observable = CommitCrawler.getObservable(testRepo.git, repo)
8386
FactHasher(localRepo, repo, mockApi)
84-
.updateFromObservable(observable)
87+
.updateFromObservable(observable, { e -> errors.add(e) })
8588

89+
assertEquals(0, errors.size)
8690
assertEquals(5, facts.size)
8791
assertTrue(facts.contains(Fact(repo, FactCodes.COMMITS_DAY_TIME, 18,
8892
1.0, author2)))

0 commit comments

Comments
 (0)