From facf3546dde12dfe8dd8027239d5fc7024271384 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Thu, 14 Sep 2023 02:16:34 +0200 Subject: [PATCH] chore:minor improvements (#222) * cleaned up more improper logging calls * made sure that all db operations are in the same place * renamed a few things to be closer to what they actually are * extracted the db-intalisation to a separate function * migrated some of the if-error statements to this syntax * minor whitespace refactorings * unnessesary `status.Errorf` instead of `status.Error` --- client/localServer/client.go | 8 +- client/publicServer/client.go | 5 +- .../backend/cafeteriaRatingDBInitializer.go | 6 +- server/backend/cafeteriaService.go | 64 ++++++------- server/backend/campus_api/campusApi.go | 9 +- server/backend/cron/canteenHeadCount.go | 13 +-- .../ios_apns/iosAPNsRepository.go | 14 +-- .../ios_apns/ios_apns_jwt/iosAPNsToken.go | 15 ++- .../ios_crypto/encryptedString.go | 4 - .../iosRequestResponseService.go | 23 ++--- server/backend/rpcserver.go | 7 +- server/main.go | 92 +++++++++---------- server/model/ios_grade.go | 8 +- 13 files changed, 111 insertions(+), 157 deletions(-) diff --git a/client/localServer/client.go b/client/localServer/client.go index 19a321bd..813c8ae7 100644 --- a/client/localServer/client.go +++ b/client/localServer/client.go @@ -48,7 +48,7 @@ func canteenHeadCount(c pb.CampusClient, ctx context.Context) { }) if err != nil { - log.Error(err) + log.WithError(err).Error("Canteen HeadCount data request failed.") } else { log.WithField("res", res).Info("Canteen HeadCount data request successful.") } @@ -245,8 +245,7 @@ func getImageToBytes(path string) []byte { } defer func(file *os.File) { - err := file.Close() - if err != nil { + if err := file.Close(); err != nil { log.WithError(err).Error("could not close file") } }(file) @@ -291,8 +290,7 @@ func storeImage(path string, i []byte) (string, error) { log.WithError(errFile).Error("Unable to create the new testfile") } defer func(out *os.File) { - err := out.Close() - if err != nil { + if err := out.Close(); err != nil { log.WithError(err).Error("File was not closed successfully") } }(out) diff --git a/client/publicServer/client.go b/client/publicServer/client.go index f06c5038..cc5a0fbd 100644 --- a/client/publicServer/client.go +++ b/client/publicServer/client.go @@ -25,11 +25,10 @@ func main() { conn, err := grpc.Dial(address, grpc.WithTransportCredentials(creds)) if err != nil { - log.WithError(err).Fatalf("did not connect") + log.WithError(err).Fatal("did not connect") } defer func(conn *grpc.ClientConn) { - err := conn.Close() - if err != nil { + if err := conn.Close(); err != nil { log.WithError(err).Error("did not close connection") } }(conn) diff --git a/server/backend/cafeteriaRatingDBInitializer.go b/server/backend/cafeteriaRatingDBInitializer.go index 1f80d270..500a93d4 100644 --- a/server/backend/cafeteriaRatingDBInitializer.go +++ b/server/backend/cafeteriaRatingDBInitializer.go @@ -208,8 +208,7 @@ func generateNameTagListFromFile(path string) multiLanguageNameTags { log.WithError(errjson).Error("Error while reading the file.") } defer func(jsonFile *os.File) { - err := jsonFile.Close() - if err != nil { + if err := jsonFile.Close(); err != nil { log.WithError(err).Error("Error in parsing json.") } }(file) @@ -224,8 +223,7 @@ func generateRatingTagListFromFile(path string) multiLanguageTags { log.WithError(errjson).Error("Error while reading or parsing the file.") } defer func(jsonFile *os.File) { - err := jsonFile.Close() - if err != nil { + if err := jsonFile.Close(); err != nil { log.WithError(err).Error("Error in parsing json.") } }(file) diff --git a/server/backend/cafeteriaService.go b/server/backend/cafeteriaService.go index c70dd309..8e2565c0 100644 --- a/server/backend/cafeteriaService.go +++ b/server/backend/cafeteriaService.go @@ -50,7 +50,7 @@ func (s *CampusServer) GetCafeteriaRatings(_ context.Context, input *pb.Cafeteri if res.Error != nil { log.WithError(res.Error).Error("Error while querying the cafeteria with Id ", cafeteriaId) - return nil, status.Errorf(codes.Internal, "This cafeteria has not yet been rated.") + return nil, status.Error(codes.Internal, "This cafeteria has not yet been rated.") } if res.RowsAffected > 0 { @@ -157,7 +157,7 @@ func (s *CampusServer) GetDishRatings(_ context.Context, input *pb.DishRatingReq if err.Error != nil { fields := log.Fields{"dishID": dishID, "cafeteriaID": cafeteriaID} log.WithError(err.Error).WithFields(fields).Error("Error while querying the average ratings") - return nil, status.Errorf(codes.Internal, "This dish has not yet been rated.") + return nil, status.Error(codes.Internal, "This dish has not yet been rated.") } if err.RowsAffected > 0 { @@ -249,15 +249,12 @@ func getImageToBytes(path string) []byte { return make([]byte, 0) } file, err := os.Open(path) - if err != nil { log.WithError(err).Error("while opening image file with path: ", path) return nil } - defer func(file *os.File) { - err := file.Close() - if err != nil { + if err := file.Close(); err != nil { log.WithError(err).Error("Unable to close the file for storing the image.") } }(file) @@ -267,8 +264,7 @@ func getImageToBytes(path string) []byte { imageAsBytes := make([]byte, size) buffer := bufio.NewReader(file) - _, err = buffer.Read(imageAsBytes) - if err != nil { + if _, err = buffer.Read(imageAsBytes); err != nil { log.WithError(err).Error("while trying to read image as bytes") return nil } @@ -376,10 +372,9 @@ func (s *CampusServer) NewCafeteriaRating(_ context.Context, input *pb.NewCafete Image: resPath, } - err := s.db.Model(&model.CafeteriaRating{}).Create(&rating).Error - if err != nil { + if err := s.db.Model(&model.CafeteriaRating{}).Create(&rating).Error; err != nil { log.WithError(err).Error("Error occurred while creating the new cafeteria rating.") - return nil, status.Errorf(codes.InvalidArgument, "Error while creating new cafeteria rating. Rating has not been saved.") + return nil, status.Error(codes.InvalidArgument, "Error while creating new cafeteria rating. Rating has not been saved.") } return storeRatingTags(s, rating.CafeteriaRating, input.RatingTags, CAFETERIA) @@ -400,12 +395,11 @@ func imageWrapper(image []byte, path string, id int32) string { } // storeImage -// stores an image and returns teh path to this image. +// stores an image and returns the path to this image. // if needed, a new directory will be created and the path is extended until it is unique func storeImage(path string, i []byte) (string, error) { if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { - err := os.MkdirAll(path, os.ModePerm) - if err != nil { + if err := os.MkdirAll(path, os.ModePerm); err != nil { log.WithError(err).WithField("path", path).Error("Directory could not be created successfully") return "", nil } @@ -430,8 +424,7 @@ func storeImage(path string, i []byte) (string, error) { return imgPath, errFile } defer func(out *os.File) { - err := out.Close() - if err != nil { + if err := out.Close(); err != nil { log.WithError(err).Error("while closing the file.") } }(out) @@ -459,7 +452,7 @@ func (s *CampusServer) NewDishRating(_ context.Context, input *pb.NewDishRatingR First(&dish).Error if errDish != nil || dish == nil { log.WithError(errDish).Error("Error while creating a new dish rating.") - return nil, status.Errorf(codes.InvalidArgument, "Dish is not offered in this week in this canteen. Rating has not been saved.") + return nil, status.Error(codes.InvalidArgument, "Dish is not offered in this week in this canteen. Rating has not been saved.") } resPath := imageWrapper(input.Image, "dishes", dish.Dish) @@ -473,10 +466,9 @@ func (s *CampusServer) NewDishRating(_ context.Context, input *pb.NewDishRatingR Image: resPath, } - err := s.db.Model(&model.DishRating{}).Create(&rating).Error - if err != nil { + if err := s.db.Model(&model.DishRating{}).Create(&rating).Error; err != nil { log.WithError(err).Error("while creating a new dish rating.") - return nil, status.Errorf(codes.Internal, "Error while creating the new rating in the database. Rating has not been saved.") + return nil, status.Error(codes.Internal, "Error while creating the new rating in the database. Rating has not been saved.") } assignDishNameTag(s, rating, dish.Dish) @@ -511,24 +503,24 @@ func assignDishNameTag(s *CampusServer, rating model.DishRating, dishID int32) { // Additionally, queries the cafeteria ID, since it checks whether the cafeteria actually exists. func inputSanitizationForNewRatingElements(rating int32, comment string, cafeteriaName string, s *CampusServer) (int32, error) { if rating > 5 || rating < 0 { - return -1, status.Errorf(codes.InvalidArgument, "Rating must be a positive number not larger than 10. Rating has not been saved.") + return -1, status.Error(codes.InvalidArgument, "Rating must be a positive number not larger than 10. Rating has not been saved.") } if len(comment) > 256 { - return -1, status.Errorf(codes.InvalidArgument, "Ratings can only contain up to 256 characters, this is too long. Rating has not been saved.") + return -1, status.Error(codes.InvalidArgument, "Ratings can only contain up to 256 characters, this is too long. Rating has not been saved.") } if strings.Contains(comment, "@") { - return -1, status.Errorf(codes.InvalidArgument, "Comments must not contain @ symbols in order to prevent misuse. Rating has not been saved.") + return -1, status.Error(codes.InvalidArgument, "Comments must not contain @ symbols in order to prevent misuse. Rating has not been saved.") } var result *model.Cafeteria res := s.db.Model(&model.Cafeteria{}). Where("name LIKE ?", cafeteriaName). First(&result) - if res.Error == gorm.ErrRecordNotFound || res.RowsAffected == 0 { + if errors.Is(res.Error, gorm.ErrRecordNotFound) || res.RowsAffected == 0 { log.WithError(res.Error).Error("Error while querying the cafeteria id by name: ", cafeteriaName) - return -1, status.Errorf(codes.InvalidArgument, "Cafeteria does not exist. Rating has not been saved.") + return -1, status.Error(codes.InvalidArgument, "Cafeteria does not exist. Rating has not been saved.") } return result.Cafeteria, nil @@ -587,11 +579,11 @@ func storeRatingTags(s *CampusServer, parentRatingID int32, tags []*pb.RatingTag } if len(errorOccurred) > 0 && len(warningOccurred) > 0 { - return &emptypb.Empty{}, status.Errorf(codes.InvalidArgument, "The tag(s) "+errorOccurred+" does not exist. Remaining rating was saved without this rating tag. The tag(s) "+warningOccurred+" occurred more than once in this rating.") + return &emptypb.Empty{}, status.Error(codes.InvalidArgument, fmt.Sprintf("The tag(s) %s does not exist. Remaining rating was saved without this rating tag. The tag(s) %s occurred more than once in this rating.", errorOccurred, warningOccurred)) } else if len(errorOccurred) > 0 { - return &emptypb.Empty{}, status.Errorf(codes.InvalidArgument, "The tag(s) "+errorOccurred+" does not exist. Remaining rating was saved without this rating tag.") + return &emptypb.Empty{}, status.Error(codes.InvalidArgument, fmt.Sprintf("The tag(s) %s does not exist. Remaining rating was saved without this rating tag.", errorOccurred)) } else if len(warningOccurred) > 0 { - return &emptypb.Empty{}, status.Errorf(codes.InvalidArgument, "The tag(s) "+warningOccurred+" occurred more than once in this rating.") + return &emptypb.Empty{}, status.Error(codes.InvalidArgument, fmt.Sprintf("The tag(s) %s occurred more than once in this rating.", warningOccurred)) } else { return &emptypb.Empty{}, nil } @@ -641,7 +633,7 @@ func (s *CampusServer) GetAvailableDishTags(_ context.Context, _ *emptypb.Empty) err := s.db.Model(&model.DishRatingTagOption{}).Select("DE as de, EN as en, dishRatingTagOption as TagId").Find(&result).Error if err != nil { log.WithError(err).Error("while loading Cafeterias from database.") - requestStatus = status.Errorf(codes.Internal, "Available dish tags could not be loaded from the database.") + requestStatus = status.Error(codes.Internal, "Available dish tags could not be loaded from the database.") } return &pb.GetTagsReply{ @@ -657,7 +649,7 @@ func (s *CampusServer) GetNameTags(_ context.Context, _ *emptypb.Empty) (*pb.Get err := s.db.Model(&model.DishNameTagOption{}).Select("DE as de, EN as en, dishNameTagOption as TagId").Find(&result).Error if err != nil { log.WithError(err).Error("while loading available Name Tags from database.") - requestStatus = status.Errorf(codes.Internal, "Available dish tags could not be loaded from the database.") + requestStatus = status.Error(codes.Internal, "Available dish tags could not be loaded from the database.") } return &pb.GetTagsReply{ @@ -673,7 +665,7 @@ func (s *CampusServer) GetAvailableCafeteriaTags(_ context.Context, _ *emptypb.E err := s.db.Model(&model.CafeteriaRatingTagOption{}).Select("DE as de, EN as en, cafeteriaRatingsTagOption as TagId").Find(&result).Error if err != nil { log.WithError(err).Error("while loading Cafeterias from database.") - requestStatus = status.Errorf(codes.Internal, "Available cafeteria tags could not be loaded from the database.") + requestStatus = status.Error(codes.Internal, "Available cafeteria tags could not be loaded from the database.") } return &pb.GetTagsReply{ @@ -689,7 +681,7 @@ func (s *CampusServer) GetCafeterias(_ context.Context, _ *emptypb.Empty) (*pb.G err := s.db.Model(&model.Cafeteria{}).Select("cafeteria as id,address,latitude,longitude").Scan(&result).Error if err != nil { log.WithError(err).Error("while loading Cafeterias from database.") - requestStatus = status.Errorf(codes.Internal, "Cafeterias could not be loaded from the database.") + requestStatus = status.Error(codes.Internal, "Cafeterias could not be loaded from the database.") } return &pb.GetCafeteriaReply{ @@ -699,13 +691,13 @@ func (s *CampusServer) GetCafeterias(_ context.Context, _ *emptypb.Empty) (*pb.G func (s *CampusServer) GetDishes(_ context.Context, request *pb.GetDishesRequest) (*pb.GetDishesReply, error) { if request.Year < 2022 { - return &pb.GetDishesReply{}, status.Errorf(codes.Internal, "Years must be larger or equal to 2022 ") // currently, no previous values have been added + return &pb.GetDishesReply{}, status.Error(codes.Internal, "Years must be larger or equal to 2022 ") // currently, no previous values have been added } if request.Week < 1 || request.Week > 53 { - return &pb.GetDishesReply{}, status.Errorf(codes.Internal, "Weeks must be in the range 1 - 53") + return &pb.GetDishesReply{}, status.Error(codes.Internal, "Weeks must be in the range 1 - 53") } if request.Day < 0 || request.Day > 4 { - return &pb.GetDishesReply{}, status.Errorf(codes.Internal, "Days must be in the range 1 (Monday) - 4 (Friday)") + return &pb.GetDishesReply{}, status.Error(codes.Internal, "Days must be in the range 1 (Monday) - 4 (Friday)") } var requestStatus error = nil @@ -721,7 +713,7 @@ func (s *CampusServer) GetDishes(_ context.Context, request *pb.GetDishesRequest if err != nil { log.WithError(err).Error("while loading Cafeterias from database.") - requestStatus = status.Errorf(codes.Internal, "Cafeterias could not be loaded from the database.") + requestStatus = status.Error(codes.Internal, "Cafeterias could not be loaded from the database.") } return &pb.GetDishesReply{ diff --git a/server/backend/campus_api/campusApi.go b/server/backend/campus_api/campusApi.go index 1a3d192d..23d6754e 100644 --- a/server/backend/campus_api/campusApi.go +++ b/server/backend/campus_api/campusApi.go @@ -38,23 +38,18 @@ func FetchGrades(token string) (*model.IOSGrades, error) { req.URL.RawQuery = q.Encode() resp, err := http.DefaultClient.Do(req) - if err != nil { log.WithError(err).Error("failed to fetch grades") return nil, ErrWhileFetchingGrades } - defer func(Body io.ReadCloser) { - err := Body.Close() - if err != nil { + if err := Body.Close(); err != nil { log.WithError(err).Error("Could not close body") } }(resp.Body) var grades model.IOSGrades - err = xml.NewDecoder(resp.Body).Decode(&grades) - - if err != nil { + if err = xml.NewDecoder(resp.Body).Decode(&grades); err != nil { log.WithError(err).Error("could not unmarshall grades") return nil, ErrorWhileUnmarshalling } diff --git a/server/backend/cron/canteenHeadCount.go b/server/backend/cron/canteenHeadCount.go index 5f42ddfe..1eecc9c2 100644 --- a/server/backend/cron/canteenHeadCount.go +++ b/server/backend/cron/canteenHeadCount.go @@ -189,8 +189,7 @@ func updateDb(canteen *CanteenApInformation, count uint32, db *gorm.DB) error { } if res.RowsAffected == 0 { - err := db.Create(&entry).Error - if err != nil { + if err := db.Create(&entry).Error; err != nil { fields := log.Fields{ "CanteenId": entry.CanteenId, "Count": entry.Count, @@ -198,8 +197,8 @@ func updateDb(canteen *CanteenApInformation, count uint32, db *gorm.DB) error { "Percent": entry.Percent, "Timestamp": entry.Timestamp} log.WithError(res.Error).WithFields(fields).Error("could not create headcount entry") + return err } - return err } return nil } @@ -216,17 +215,15 @@ func (canteen CanteenApInformation) requestApData() []AccessPoint { // Ensure we close the body once we leave this function if resp.Body != nil { defer func(Body io.ReadCloser) { - err := Body.Close() - if err != nil { + if err := Body.Close(); err != nil { log.WithError(err).Error("Could not close body") } }(resp.Body) } // Parse as JSON - aps := []AccessPoint{} - err = json.NewDecoder(resp.Body).Decode(&aps) - if err != nil { + var aps []AccessPoint + if err = json.NewDecoder(resp.Body).Decode(&aps); err != nil { log.WithError(err).Error("Canteen HeadCount parsing output as JSON failed for: ", canteen.CanteenId) return []AccessPoint{} } diff --git a/server/backend/ios_notifications/ios_apns/iosAPNsRepository.go b/server/backend/ios_notifications/ios_apns/iosAPNsRepository.go index 1091b8f7..338b0d31 100644 --- a/server/backend/ios_notifications/ios_apns/iosAPNsRepository.go +++ b/server/backend/ios_notifications/ios_apns/iosAPNsRepository.go @@ -100,20 +100,18 @@ func (r *Repository) SendNotification(notification *model.IOSNotificationPayload resp, err := client.Do(req) if err != nil { - log.Error(err) + log.WithError(err).Error("Could not send notification") return nil, ErrCouldNotSendNotification } - defer func(Body io.ReadCloser) { - err := Body.Close() - if err != nil { + if err := Body.Close(); err != nil { log.WithError(err).Error("Could not close body") } }(resp.Body) var response model.IOSRemoteNotificationResponse if err = json.NewDecoder(resp.Body).Decode(&response); err != nil && err != io.EOF { - log.Error(err) + log.WithError(err).Error("Could not decode APNs response") return nil, ErrCouldNotDecodeAPNsResponse } @@ -137,15 +135,13 @@ func NewRepository(db *gorm.DB, token *ios_apns_jwt.Token) *Repository { func NewCronRepository(db *gorm.DB) (*Repository, error) { if err := ValidateRequirementsForIOSNotificationsService(); err != nil { - log.Warn(err) - + log.WithError(err).Warn("Failed to validate requirements for ios notifications service") return nil, err } token, err := ios_apns_jwt.NewToken() - if err != nil { - log.Fatal(err) + log.WithError(err).Fatal("Could not create APNs token") } return NewRepository(db, token), nil diff --git a/server/backend/ios_notifications/ios_apns/ios_apns_jwt/iosAPNsToken.go b/server/backend/ios_notifications/ios_apns/ios_apns_jwt/iosAPNsToken.go index 0b6fe133..bf00d984 100644 --- a/server/backend/ios_notifications/ios_apns/ios_apns_jwt/iosAPNsToken.go +++ b/server/backend/ios_notifications/ios_apns/ios_apns_jwt/iosAPNsToken.go @@ -39,7 +39,6 @@ type Token struct { func NewToken() (*Token, error) { encryptionKey, err := APNsEncryptionKeyFromFile() - if err != nil { return nil, err } @@ -50,9 +49,7 @@ func NewToken() (*Token, error) { TeamId: ApnsTeamId, } - _, err = token.Generate() - - if err != nil { + if err = token.Generate(); err != nil { return nil, err } @@ -106,7 +103,7 @@ func (t *Token) GenerateNewTokenIfExpired() (bearer string) { defer t.Unlock() if t.IsExpired() { - _, err := t.Generate() + err := t.Generate() if err != nil { return "" } @@ -119,9 +116,9 @@ func (t *Token) IsExpired() bool { return currentTimestamp() >= (t.IssuedAt + TokenTimeout) } -func (t *Token) Generate() (bool, error) { +func (t *Token) Generate() error { if t.EncryptionKey == nil { - return false, ErrorAuthKeyNil + return ErrorAuthKeyNil } issuedAt := currentTimestamp() @@ -141,13 +138,13 @@ func (t *Token) Generate() (bool, error) { token, err := jwtToken.SignedString(t.EncryptionKey) if err != nil { - return false, err + return err } t.IssuedAt = issuedAt t.Bearer = token - return true, nil + return nil } func currentTimestamp() int64 { diff --git a/server/backend/ios_notifications/ios_crypto/encryptedString.go b/server/backend/ios_notifications/ios_crypto/encryptedString.go index 24a04a53..5ee0aa4b 100644 --- a/server/backend/ios_notifications/ios_crypto/encryptedString.go +++ b/server/backend/ios_notifications/ios_crypto/encryptedString.go @@ -97,19 +97,16 @@ func SymmetricEncrypt(plaintext string, key string) (*EncryptedString, error) { func SymmetricDecrypt(encryptedString EncryptedString, key string) (string, error) { bytesKey := []byte(key) bytesEncryptedString, err := base64.StdEncoding.DecodeString(string(encryptedString)) - if err != nil { return "", err } c, err := aes.NewCipher(bytesKey) - if err != nil { return "", err } gcm, err := cipher.NewGCM(c) - if err != nil { return "", err } @@ -118,7 +115,6 @@ func SymmetricDecrypt(encryptedString EncryptedString, key string) (string, erro nonce, ciphertext := bytesEncryptedString[:nonceSize], bytesEncryptedString[nonceSize:] plaintext, err := gcm.Open(nil, nonce, ciphertext, nil) - if err != nil { return "", err } diff --git a/server/backend/ios_notifications/ios_request_response/iosRequestResponseService.go b/server/backend/ios_notifications/ios_request_response/iosRequestResponseService.go index b5a233f0..cbd57102 100644 --- a/server/backend/ios_notifications/ios_request_response/iosRequestResponseService.go +++ b/server/backend/ios_notifications/ios_request_response/iosRequestResponseService.go @@ -76,19 +76,19 @@ func (service *Service) handleDeviceCampusTokenRequest(requestLog *model.IOSDevi apiGrades, err := campus_api.FetchGrades(campusToken) if err != nil { - log.Error("Could not fetch grades: ", err) + log.WithError(err).Error("Could not fetch grades") return nil, ErrInternalHandleGrades } oldEncryptedGrades, err := service.Repository.GetIOSEncryptedGrades(requestLog.DeviceID) if err != nil { - log.Error("Could not get old grades: ", err) + log.WithError(err).Error("Could not get old grades") return nil, ErrInternalHandleGrades } oldGrades, err := decryptGrades(oldEncryptedGrades, campusToken) if err != nil { - log.Error("Could not decrypt old grades: ", err) + log.WithError(err).Error("Could not decrypt old grades") return nil, ErrInternalHandleGrades } @@ -104,7 +104,7 @@ func (service *Service) handleDeviceCampusTokenRequest(requestLog *model.IOSDevi err = service.Repository.DeleteEncryptedGrades(requestLog.DeviceID) if err != nil { - log.Error("Could not delete old grades: ", err) + log.WithError(err).Error("Could not delete old grades") return nil, ErrInternalHandleGrades } @@ -129,7 +129,7 @@ func (service *Service) deleteRequestLog(requestLog *model.IOSDeviceRequestLog) err := service.Repository.DeleteAllRequestLogsForThisDeviceWithType(requestLog) if err != nil { - log.Error("Could not delete request logs: ", err) + log.WithError(err).Error("Could not delete request logs") } } @@ -139,7 +139,7 @@ func decryptGrades(grades []model.IOSEncryptedGrade, campusToken string) ([]mode err := encryptedGrade.Decrypt(campusToken) if err != nil { - log.Error("Could not decrypt grade: ", err) + log.WithError(err).Error("Could not decrypt grade") return nil, status.Error(codes.Internal, "Could not decrypt grade") } @@ -177,15 +177,13 @@ func (service *Service) encryptGradesAndStoreInDatabase(grades []model.IOSGrade, } err := encryptedGrade.Encrypt(campusToken) - if err != nil { - log.Error("Could not encrypt grade: ", err) + log.WithError(err).Error("Could not encrypt grade") } err = service.Repository.SaveEncryptedGrade(&encryptedGrade) - if err != nil { - log.Error("Could not save grade: ", err) + log.WithError(err).Error("Could not save grade") } } } @@ -210,12 +208,11 @@ func sendGradesToDevice(device *model.IOSDevice, grades []model.IOSGrade, apns * Alert(alertTitle, "", alertBody). Encrypt(device.PublicKey) - log.WithField("DeviceID", device.DeviceID).Infof("Sending push notification") + log.WithField("DeviceID", device.DeviceID).Info("Sending push notification") _, err := apns.SendAlertNotification(notificationPayload) - if err != nil { - log.Error("Could not send notification: ", err) + log.WithField("DeviceID", device.DeviceID).WithError(err).Error("Could not send notification") } } diff --git a/server/backend/rpcserver.go b/server/backend/rpcserver.go index 88b10b55..5201d289 100644 --- a/server/backend/rpcserver.go +++ b/server/backend/rpcserver.go @@ -19,7 +19,7 @@ func (s *CampusServer) GRPCServe(l net.Listener) error { grpcServer := grpc.NewServer() pb.RegisterCampusServer(grpcServer, s) if err := grpcServer.Serve(l); err != nil { - log.Fatalf("failed to serve: %v", err) + log.WithError(err).Fatal("failed to serve") } return grpcServer.Serve(l) } @@ -47,7 +47,7 @@ func New(db *gorm.DB) *CampusServer { func NewIOSNotificationsService() *IOSNotificationsService { if err := ios_apns.ValidateRequirementsForIOSNotificationsService(); err != nil { - log.Warn(err) + log.WithError(err).Warn("failed to validate requirements for ios notifications service") return &IOSNotificationsService{ APNSToken: nil, @@ -56,9 +56,8 @@ func NewIOSNotificationsService() *IOSNotificationsService { } token, err := ios_apns_jwt.NewToken() - if err != nil { - log.Fatal(err) + log.WithError(err).Fatal("failed to create new token") } return &IOSNotificationsService{ diff --git a/server/main.go b/server/main.go index f6d74a78..e7845af6 100644 --- a/server/main.go +++ b/server/main.go @@ -45,18 +45,6 @@ func main() { setupTelemetry() defer sentry.Flush(2 * time.Second) // make sure that sentry handles shutdowns gracefully - // Connect to DB - var conn gorm.Dialector - shouldAutoMigrate := false - dbHost := os.Getenv("DB_DSN") - if dbHost != "" { - log.Info("Connecting to dsn") - conn = mysql.Open(dbHost) - } else { - log.Error("Failed to start! The 'DB_DSN' environment variable is not defined. Take a look at the README.md for more details.") - os.Exit(-1) - } - // initializing connection to InfluxDB err := backend.ConnectToInfluxDB() if errors.Is(err, backend.ErrInfluxTokenNotConfigured) { @@ -67,18 +55,7 @@ func main() { log.WithError(err).Error("InfluxDB connection failed - health check failed") } - db, err := gorm.Open(conn, &gorm.Config{}) - if err != nil { - panic("failed to connect database") - } - - // Migrate the schema - tumMigrator := migration.New(db, shouldAutoMigrate) - err = tumMigrator.Migrate() - if err != nil { - log.WithError(err).Fatal("Failed to migrate database") - return - } + db := setupDB() // Create any other background services (these shouldn't do any long-running work here) cronService := cron.New(db) @@ -87,29 +64,29 @@ func main() { // Listen to our configured ports listener, err := net.Listen("tcp", httpPort) if err != nil { - log.Fatalf("failed to listen: %v", err) + log.WithError(err).Fatal("failed to listen") } - m := cmux.New(listener) - grpcListener := m.MatchWithWriters(cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc")) - httpListener := m.Match(cmux.HTTP1Fast()) + mux := cmux.New(listener) + grpcListener := mux.MatchWithWriters(cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc")) + httpListener := mux.Match(cmux.HTTP1Fast()) // HTTP Stuff - mux := http.NewServeMux() - httpServer := &http.Server{Handler: mux} - mux.HandleFunc("/imprint", func(w http.ResponseWriter, r *http.Request) { + httpMux := http.NewServeMux() + httpServer := &http.Server{Handler: httpMux} + httpMux.HandleFunc("/imprint", func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("Hello, world!")) }) - mux.HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) { + httpMux.HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("healthy")) }) static, _ := fs.Sub(swagfs, "swagger") - mux.Handle("/", http.FileServer(http.FS(static))) + httpMux.Handle("/", http.FileServer(http.FS(static))) // Main GRPC Server - grpcS := grpc.NewServer() - pb.RegisterCampusServer(grpcS, campusService) + grpcServer := grpc.NewServer() + pb.RegisterCampusServer(grpcServer, campusService) // GRPC Gateway for HTTP REST -> GRPC grpcGatewayMux := runtime.NewServeMux(runtime.WithIncomingHeaderMatcher( @@ -123,31 +100,52 @@ func main() { }), runtime.WithErrorHandler(errorHandler), ) - ctx := context.Background() opts := []grpc.DialOption{ grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUserAgent("internal"), grpc.WithUnaryInterceptor(addMethodNameInterceptor), } - if err := pb.RegisterCampusHandlerFromEndpoint(ctx, grpcGatewayMux, httpPort, opts); err != nil { - panic(err) + if err := pb.RegisterCampusHandlerFromEndpoint(context.Background(), grpcGatewayMux, httpPort, opts); err != nil { + log.WithError(err).Panic("could not RegisterCampusHandlerFromEndpoint") } - restPrefix := "/v1" - mux.Handle("/v1/", http.StripPrefix(restPrefix, grpcGatewayMux)) + httpMux.Handle("/v1/", http.StripPrefix("/v1", grpcGatewayMux)) // Start each server in its own go routine and logs any errors g := errgroup.Group{} - g.Go(func() error { return grpcS.Serve(grpcListener) }) + g.Go(func() error { return grpcServer.Serve(grpcListener) }) g.Go(func() error { return httpServer.Serve(httpListener) }) - g.Go(func() error { return m.Serve() }) + g.Go(func() error { return mux.Serve() }) g.Go(func() error { return cronService.Run() }) // Setup cron jobs g.Go(func() error { return campusService.RunDeviceFlusher() }) // Setup campus service log.Info("running server") - err = g.Wait() + if err := g.Wait(); err != nil { + log.WithError(err).Error("encountered issue while running the server") + } +} + +// setupDB connects to the database and migrates it if necessary +func setupDB() *gorm.DB { + // Connect to DB + var conn gorm.Dialector + if dbHost := os.Getenv("DB_DSN"); dbHost == "" { + log.Fatal("Failed to start! The 'DB_DSN' environment variable is not defined. Take a look at the README.md for more details.") + } else { + log.Info("Connecting to dsn") + conn = mysql.Open(dbHost) + } + + db, err := gorm.Open(conn, &gorm.Config{}) if err != nil { - log.Error(err) + log.WithError(err).Panic("failed to connect database") + } + + // Migrate the schema + // currently not activated as + if err := migration.New(db, false).Migrate(); err != nil { + log.WithError(err).Fatal("Failed to migrate database") } + return db } // setupTelemetry initializes our telemetry stack @@ -172,7 +170,7 @@ func setupTelemetry() { }); err != nil { log.WithError(err).Error("Sentry initialization failed") } - log.AddHook(sentryhook.New([]log.Level{log.PanicLevel, log.FatalLevel, log.ErrorLevel})) + log.AddHook(sentryhook.New([]log.Level{log.PanicLevel, log.FatalLevel, log.ErrorLevel, log.WarnLevel})) } else { log.Info("continuing without sentry") } @@ -221,9 +219,7 @@ func errorHandler(_ context.Context, _ *runtime.ServeMux, _ runtime.Marshaler, w w.Header().Set("Content-Type", "application/json") w.WriteHeader(errorResp.StatusCode) - err = json.NewEncoder(w).Encode(errorResp) - - if err != nil { + if err = json.NewEncoder(w).Encode(errorResp); err != nil { log.WithError(err).Error("Marshal error response failed") return } diff --git a/server/model/ios_grade.go b/server/model/ios_grade.go index 4d6d282a..5bd8c31b 100644 --- a/server/model/ios_grade.go +++ b/server/model/ios_grade.go @@ -33,13 +33,11 @@ type customDate struct { func (c *customDate) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { var v string - err := d.DecodeElement(&v, &start) - if err != nil { + if err := d.DecodeElement(&v, &start); err != nil { return err } t, err := time.Parse("2006-01-02", v) - if err != nil { return err } @@ -66,13 +64,11 @@ type IOSEncryptedGrade struct { func (e *IOSEncryptedGrade) Encrypt(key string) error { encryptedTitle, err := ios_crypto.SymmetricEncrypt(e.LectureTitle, key) - if err != nil { return err } encryptedGrade, err := ios_crypto.SymmetricEncrypt(e.Grade, key) - if err != nil { return err } @@ -86,13 +82,11 @@ func (e *IOSEncryptedGrade) Encrypt(key string) error { func (e *IOSEncryptedGrade) Decrypt(key string) error { decryptedTitle, err := ios_crypto.SymmetricDecrypt(ios_crypto.EncryptedString(e.LectureTitle), key) - if err != nil { return err } decryptedGrade, err := ios_crypto.SymmetricDecrypt(ios_crypto.EncryptedString(e.Grade), key) - if err != nil { return err }