Skip to content

Commit b4c6d51

Browse files
mwfarbmwfarb
authored
Improved webapp DB error handling (#61)
* cleanup go code missed from last cleanup * improved db error handling - Refactor all db error handling to propagate errors out of models package. - Alter handling of segments to allow paths to still render if segment data is unavailable. - Closes #20, since non-critical panics are removed. * fixed handling of empty paths
1 parent a9872da commit b4c6d51

File tree

8 files changed

+296
-237
lines changed

8 files changed

+296
-237
lines changed

‎webapp/lib/bwcont.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,16 @@ func GetBwByTimeHandler(w http.ResponseWriter, r *http.Request, active bool, src
141141
since := r.PostFormValue("since")
142142
log.Info("Requesting data since", "timestamp", since)
143143
// find undisplayed test results
144-
bwTestResults := model.ReadBwTestItemsSince(since)
144+
bwTestResults, err := model.ReadBwTestItemsSince(since)
145+
if CheckError(err) {
146+
returnError(w, err)
147+
return
148+
}
145149
log.Debug("Requested data:", "bwTestResults", bwTestResults)
146150

147151
bwtestsJSON, err := json.Marshal(bwTestResults)
148152
if CheckError(err) {
153+
returnError(w, err)
149154
return
150155
}
151156
jsonBuf := []byte(`{ "graph": ` + string(bwtestsJSON))

‎webapp/lib/sciond.go‎

Lines changed: 78 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,22 @@ func returnError(w http.ResponseWriter, err error) {
3838
fmt.Fprintf(w, `{"err":`+strconv.Quote(err.Error())+`}`)
3939
}
4040

41+
func returnPathHandler(w http.ResponseWriter, pathJson []byte, segJson []byte, err error) {
42+
var buffer bytes.Buffer
43+
buffer.WriteString(`{"src":"sciond"`)
44+
if pathJson != nil {
45+
buffer.WriteString(fmt.Sprintf(`,"paths":%s`, pathJson))
46+
}
47+
if segJson != nil {
48+
buffer.WriteString(fmt.Sprintf(`,"segments":%s`, segJson))
49+
}
50+
if err != nil {
51+
buffer.WriteString(fmt.Sprintf(`,"err":%s`, strconv.Quote(err.Error())))
52+
}
53+
buffer.WriteString(`}`)
54+
fmt.Fprintf(w, buffer.String())
55+
}
56+
4157
// sciond data sources and calls
4258

4359
// PathTopoHandler handles requests for paths, returning results from sciond.
@@ -78,34 +94,55 @@ func PathTopoHandler(w http.ResponseWriter, r *http.Request) {
7894
}
7995
}
8096

81-
paths := getPaths(*clientCCAddr, *serverCCAddr)
82-
if len(paths) == 0 {
83-
returnError(w, fmt.Errorf("No paths from %s to %s", clientCCAddr.IA,
84-
serverCCAddr.IA))
97+
paths, err := getPathsJSON(*clientCCAddr, *serverCCAddr)
98+
if CheckError(err) {
99+
returnError(w, err)
100+
return
101+
}
102+
log.Debug("PathTopoHandler:", "paths", string(paths))
103+
104+
// Since segments data is supplimentary to paths data, if segments data
105+
// fails, provide the error, but we must still allow paths data to return.
106+
segments, err := getSegmentsJSON(*clientCCAddr)
107+
if CheckError(err) {
108+
returnPathHandler(w, paths, nil, err)
85109
return
86110
}
111+
log.Debug("PathTopoHandler:", "segments", string(segments))
87112

88-
jsonPathInfo, _ := json.Marshal(paths)
89-
log.Debug("PathTopoHandler:", "jsonPathInfo", string(jsonPathInfo))
113+
returnPathHandler(w, paths, segments, err)
114+
}
90115

116+
func getSegmentsJSON(local snet.Addr) ([]byte, error) {
91117
// load segments from paths database
92-
var dbSrcFile = findDBFilename(clientCCAddr.IA)
93-
dbTmpFile := copyDBToTemp(dbSrcFile)
118+
var dbSrcFile = findDBFilename(local.IA)
119+
dbTmpFile, err := copyDBToTemp(dbSrcFile)
120+
if err != nil {
121+
return nil, err
122+
}
94123
// since http.ListenAndServe() blocks, ensure we generate a local db object
95124
// which will live only during the http call
96-
db := pathdb.InitDB(dbTmpFile)
125+
db, err := pathdb.InitDB(dbTmpFile)
126+
if err != nil {
127+
return nil, err
128+
}
97129
defer func() {
98130
pathdb.CloseDB(db)
99131
removeAllDir(filepath.Dir(dbTmpFile))
100132
}()
101-
segTypes := pathdb.ReadSegTypesAll(db)
102-
segments := pathdb.ReadSegmentsAll(db, segTypes)
103-
104-
jsonSegsInfo, _ := json.Marshal(segments)
105-
log.Debug("PathTopoHandler:", "jsonSegsInfo", string(jsonSegsInfo))
106-
107-
fmt.Fprintf(w, fmt.Sprintf(`{"paths":%s,"segments":%s}`,
108-
jsonPathInfo, jsonSegsInfo))
133+
segTypes, err := pathdb.ReadSegTypesAll(db)
134+
if err != nil {
135+
return nil, err
136+
}
137+
segments, err := pathdb.ReadSegmentsAll(db, segTypes)
138+
if err != nil {
139+
return nil, err
140+
}
141+
jsonSegsInfo, err := json.Marshal(segments)
142+
if err != nil {
143+
return nil, err
144+
}
145+
return jsonSegsInfo, nil
109146
}
110147

111148
func findDBFilename(ia addr.IA) string {
@@ -120,52 +157,60 @@ func findDBFilename(ia addr.IA) string {
120157
}
121158

122159
// returns the name of the created file
123-
func copyDBToTemp(filename string) string {
160+
func copyDBToTemp(filename string) (string, error) {
124161
copyOneFile := func(dstDir, srcFileName string) error {
125162
src, err := os.Open(srcFileName)
126-
if CheckError(err) {
127-
return fmt.Errorf("Cannot open %s: %v", srcFileName, err)
163+
if err != nil {
164+
return err
128165
}
129166
defer src.Close()
130167
dstFilename := filepath.Join(dstDir, filepath.Base(srcFileName))
131168
dst, err := os.Create(dstFilename)
132-
if CheckError(err) {
133-
return fmt.Errorf("Cannot open %s: %v", dstFilename, err)
169+
if err != nil {
170+
return err
134171
}
135172
defer dst.Close()
136173
_, err = io.Copy(dst, src)
137-
if CheckError(err) {
138-
return fmt.Errorf("Cannot copy %s to %s: %v", srcFileName, dstFilename, err.Error())
174+
if err != nil {
175+
return err
139176
}
140177
return nil
141178
}
142179
dirName, err := ioutil.TempDir("/tmp", "sciond_dump")
143-
if CheckError(err) {
144-
return err.Error()
180+
if err != nil {
181+
return "", err
145182
}
146-
147183
err = copyOneFile(dirName, filename)
148-
if CheckError(err) {
149-
fmt.Fprintf(os.Stderr, "No panic: %v", err)
184+
if err != nil {
185+
return "", err
150186
}
151187
err = copyOneFile(dirName, filename+"-wal")
152-
CheckError(err)
153-
return filepath.Join(dirName, filepath.Base(filename))
188+
if err != nil {
189+
return "", err
190+
}
191+
return filepath.Join(dirName, filepath.Base(filename)), nil
154192
}
155193

156194
func removeAllDir(dirName string) {
157195
err := os.RemoveAll(dirName)
158196
CheckError(err)
159197
}
160198

161-
func getPaths(local snet.Addr, remote snet.Addr) []*spathmeta.AppPath {
199+
func getPathsJSON(local snet.Addr, remote snet.Addr) ([]byte, error) {
162200
pathMgr := snet.DefNetwork.PathResolver()
163201
pathSet := pathMgr.Query(context.Background(), local.IA, remote.IA)
202+
if len(pathSet) == 0 {
203+
return nil, fmt.Errorf("No paths from %s to %s", local.IA, remote.IA)
204+
}
164205
var appPaths []*spathmeta.AppPath
165206
for _, path := range pathSet {
166207
appPaths = append(appPaths, path)
167208
}
168-
return appPaths
209+
jsonPathInfo, err := json.Marshal(appPaths)
210+
if err != nil {
211+
return nil, err
212+
}
213+
return jsonPathInfo, nil
169214
}
170215

171216
// AsTopoHandler handles requests for AS data, returning results from sciond.

‎webapp/models/bwtests.go‎

Lines changed: 29 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ type BwTestGraph struct {
8282
}
8383

8484
// createBwTestTable operates on the DB to create the bwtests table.
85-
func createBwTestTable() {
85+
func createBwTestTable() error {
8686
sqlCreateTable := `
8787
CREATE TABLE IF NOT EXISTS bwtests(
8888
Inserted BIGINT NOT NULL PRIMARY KEY,
@@ -115,13 +115,11 @@ func createBwTestTable() {
115115
);
116116
`
117117
_, err := db.Exec(sqlCreateTable)
118-
if CheckError(err) {
119-
panic(err)
120-
}
118+
return err
121119
}
122120

123121
// StoreBwTestItem operates on the DB to insert a BwTestItem.
124-
func StoreBwTestItem(bwtest *BwTestItem) {
122+
func StoreBwTestItem(bwtest *BwTestItem) error {
125123
sqlInsert := `
126124
INSERT INTO bwtests(
127125
Inserted,
@@ -156,12 +154,12 @@ func StoreBwTestItem(bwtest *BwTestItem) {
156154
) values(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
157155
`
158156
stmt, err := db.Prepare(sqlInsert)
159-
if CheckError(err) {
160-
panic(err)
157+
if err != nil {
158+
return err
161159
}
162160
defer stmt.Close()
163161

164-
_, err2 := stmt.Exec(
162+
_, err = stmt.Exec(
165163
bwtest.Inserted,
166164
bwtest.ActualDuration,
167165
bwtest.CIa,
@@ -191,13 +189,11 @@ func StoreBwTestItem(bwtest *BwTestItem) {
191189
bwtest.Error,
192190
bwtest.Path,
193191
bwtest.Log)
194-
if CheckError(err2) {
195-
panic(err2)
196-
}
192+
return err
197193
}
198194

199195
// ReadBwTestItemsAll operates on the DB to return all bwtests rows.
200-
func ReadBwTestItemsAll() []BwTestItem {
196+
func ReadBwTestItemsAll() ([]BwTestItem, error) {
201197
sqlReadAll := `
202198
SELECT
203199
Inserted,
@@ -233,15 +229,15 @@ func ReadBwTestItemsAll() []BwTestItem {
233229
ORDER BY datetime(Inserted) DESC
234230
`
235231
rows, err := db.Query(sqlReadAll)
236-
if CheckError(err) {
237-
panic(err)
232+
if err != nil {
233+
return nil, err
238234
}
239235
defer rows.Close()
240236

241237
var result []BwTestItem
242238
for rows.Next() {
243239
bwtest := BwTestItem{}
244-
err2 := rows.Scan(
240+
err = rows.Scan(
245241
&bwtest.Inserted,
246242
&bwtest.ActualDuration,
247243
&bwtest.CIa,
@@ -271,17 +267,17 @@ func ReadBwTestItemsAll() []BwTestItem {
271267
&bwtest.Error,
272268
&bwtest.Path,
273269
&bwtest.Log)
274-
if CheckError(err2) {
275-
panic(err2)
270+
if err != nil {
271+
return nil, err
276272
}
277273
result = append(result, bwtest)
278274
}
279-
return result
275+
return result, nil
280276
}
281277

282278
// ReadBwTestItemsSince operates on the DB to return all bwtests rows
283279
// which are more recent than the 'since' epoch in ms.
284-
func ReadBwTestItemsSince(since string) []BwTestGraph {
280+
func ReadBwTestItemsSince(since string) ([]BwTestGraph, error) {
285281
sqlReadSince := `
286282
SELECT
287283
Inserted,
@@ -298,15 +294,15 @@ func ReadBwTestItemsSince(since string) []BwTestGraph {
298294
ORDER BY datetime(Inserted) DESC
299295
`
300296
rows, err := db.Query(sqlReadSince, since)
301-
if CheckError(err) {
302-
panic(err)
297+
if err != nil {
298+
return nil, err
303299
}
304300
defer rows.Close()
305301

306302
var result []BwTestGraph
307303
for rows.Next() {
308304
bwtest := BwTestGraph{}
309-
err2 := rows.Scan(
305+
err = rows.Scan(
310306
&bwtest.Inserted,
311307
&bwtest.ActualDuration,
312308
&bwtest.CSBandwidth,
@@ -316,38 +312,39 @@ func ReadBwTestItemsSince(since string) []BwTestGraph {
316312
&bwtest.Error,
317313
&bwtest.Path,
318314
&bwtest.Log)
319-
if CheckError(err2) {
320-
panic(err2)
315+
if err != nil {
316+
return nil, err
321317
}
322318
result = append(result, bwtest)
323319
}
324-
return result
320+
return result, nil
325321
}
326322

327323
// DeleteBwTestItemsBefore operates on the DB to remote all bwtests rows
328324
// which are more older than the 'before' epoch in ms.
329-
func DeleteBwTestItemsBefore(before string) int64 {
325+
func DeleteBwTestItemsBefore(before string) (int64, error) {
330326
sqlDeleteBefore := `
331327
DELETE FROM bwtests
332328
WHERE Inserted < ?
333329
`
334330
res, err := db.Exec(sqlDeleteBefore, before)
335-
if CheckError(err) {
336-
panic(err)
331+
if err != nil {
332+
return 0, err
337333
}
338334
count, err := res.RowsAffected()
339-
if CheckError(err) {
340-
panic(err)
335+
if err != nil {
336+
return count, err
341337
}
342-
return count
338+
return count, nil
343339
}
344340

345341
// MaintainDatabase is a goroutine that runs independanly to cleanup the
346342
// database according to the defined schedule.
347343
func MaintainDatabase() {
348344
for {
349345
before := time.Now().Add(-bwTestDbExpire)
350-
count := DeleteBwTestItemsBefore(strconv.FormatInt(before.UnixNano()/1e6, 10))
346+
count, err := DeleteBwTestItemsBefore(strconv.FormatInt(before.UnixNano()/1e6, 10))
347+
CheckError(err)
351348
if count > 0 {
352349
log.Warn(fmt.Sprint("Deleting", count, "bwtests db rows older than", bwTestDbExpire))
353350
}

0 commit comments

Comments
 (0)