Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error handling in controllers #324

Open
wants to merge 11 commits into
base: development
Choose a base branch
from
Open
4 changes: 2 additions & 2 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
- name: Set up Go
uses: WillAbides/setup-go-faster@v1.14.0
with:
go-version: 1.17
go-version: 1.21

- run: ls ${{ github.workspace }}

Expand All @@ -36,7 +36,7 @@ jobs:
- name: Set up Go
uses: WillAbides/setup-go-faster@v1.14.0
with:
go-version: 1.17
go-version: 1.21

- name: Lint
uses: golangci/golangci-lint-action@v6
Expand Down
10 changes: 2 additions & 8 deletions controllers/about.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package controllers

import (
"log"
"net/http"

"github.com/UniversityRadioYork/2016-site/models"
Expand All @@ -26,18 +25,13 @@ func (aboutC *AboutController) Get(w http.ResponseWriter, r *http.Request) {
teamM := models.NewTeamModel(aboutC.session)
teams, err := teamM.GetAll()
if err != nil {
log.Println(err)
utils.RenderTemplate(w, aboutC.config.PageContext, nil, "404.tmpl")
aboutC.handleError(w, r, err, "TeamModel.GetAll")
return
}
data := struct {
Teams []myradio.Team
}{
Teams: teams,
}
err = utils.RenderTemplate(w, aboutC.config.PageContext, data, "about.tmpl")
if err != nil {
log.Println(err)
return
}
utils.RenderTemplate(w, aboutC.config.PageContext, data, "about.tmpl")
}
61 changes: 61 additions & 0 deletions controllers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,18 @@
*/

import (
"errors"
"fmt"
"log"
"net/http"
"runtime"
"strings"

"github.com/UniversityRadioYork/2016-site/structs"
"github.com/UniversityRadioYork/2016-site/utils"
"github.com/UniversityRadioYork/myradio-go"
"github.com/UniversityRadioYork/myradio-go/api"
"github.com/getsentry/sentry-go"
)

// ControllerInterface is the interface to which controllers adhere.
Expand Down Expand Up @@ -76,3 +84,56 @@
func (c *Controller) Options(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Method Not Allowed", 405)
}

// handleError converts an error into a descriptive error page. It takes care of logging it with the given context.
func (c *Controller) handleError(w http.ResponseWriter, r *http.Request, err error, context string) {
var httpErr utils.HTTPError
if errors.As(err, &httpErr) {
switch httpErr.Status {
case 404, 500: // these we have templates for
w.WriteHeader(httpErr.Status)
utils.RenderTemplate(w, c.config.PageContext, nil, fmt.Sprintf("%d.tmpl", httpErr.Status))
return
default:
http.Error(w, httpErr.Message, httpErr.Status)
return
}
}

pc, file, line, ok := runtime.Caller(1)
if ok {
fn := runtime.FuncForPC(pc)
if fn != nil {
context = fmt.Sprintf("%s at %s:%d (%s)", context, file, line, strings.Replace(fn.Name(), "github.com/UniversityRadioYork/2016-site/", "", 1))
} else {
context = fmt.Sprintf("%s at %s:%d", context, file, line)
}
}

sentry.WithScope(func(scope *sentry.Scope) {
scope.SetContext("error", map[string]any{
"context": context,
})
sentry.CaptureException(err)
})

var apiErr api.Error

Check failure on line 120 in controllers/controller.go

View workflow job for this annotation

GitHub Actions / lint

undefined: api.Error (typecheck)

Check failure on line 120 in controllers/controller.go

View workflow job for this annotation

GitHub Actions / lint

undefined: api.Error) (typecheck)

Check failure on line 120 in controllers/controller.go

View workflow job for this annotation

GitHub Actions / build

undefined: api.Error
if errors.As(err, &apiErr) {
switch apiErr.Code {
case http.StatusNotFound:
w.WriteHeader(404)
utils.RenderTemplate(w, c.config.PageContext, nil, "404.tmpl")
return
case http.StatusForbidden:
// 2016-site should never hit this, it's likely a misconfiguration of our API key's permissions
log.Printf("Received 403 from MyRadio API [%s]: %v", context, err)
default:
log.Printf("Unexpected MyRadio API error [%s]: %v", context, err)
}
} else {
log.Printf("Unexpected error [%s]: %v", context, err)
}

w.WriteHeader(500)
utils.RenderTemplate(w, c.config.PageContext, nil, "500.tmpl")
}
11 changes: 2 additions & 9 deletions controllers/getinvolved.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package controllers

import (
"log"
"net/http"
"sort"

Expand Down Expand Up @@ -54,8 +53,7 @@ func (gic *GetInvolvedController) Get(w http.ResponseWriter, r *http.Request) {
colleges, numTeams, listTeamMap, faqs, err := gim.Get()

if err != nil {
//@TODO: Do something proper here, render 404 or something
log.Println(err)
gic.handleError(w, r, err, "GetInvolvedModel.Get")
return
}

Expand All @@ -74,10 +72,5 @@ func (gic *GetInvolvedController) Get(w http.ResponseWriter, r *http.Request) {
FAQs: faqs,
}

err = utils.RenderTemplate(w, gic.config.PageContext, data, "getinvolved.tmpl")
if err != nil {
log.Println(err)
return
}

utils.RenderTemplate(w, gic.config.PageContext, data, "getinvolved.tmpl")
}
11 changes: 4 additions & 7 deletions controllers/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (ic *IndexController) Get(w http.ResponseWriter, r *http.Request) {
currentAndNext, banners, timeslots, podcasts, showOnAir, err := model.Get()

if err != nil {
log.Println(err)
ic.handleError(w, r, err, "IndexModel.Get")
return
}

Expand Down Expand Up @@ -67,7 +67,7 @@ func (ic *IndexController) Post(w http.ResponseWriter, r *http.Request) {
currentAndNext, banners, timeslots, podcasts, showOnAir, err := model.Get()

if err != nil {
log.Println(err)
ic.handleError(w, r, err, "IndexModel.Get")
return
}

Expand All @@ -84,6 +84,7 @@ func (ic *IndexController) Post(w http.ResponseWriter, r *http.Request) {
msgmodel := models.NewMessageModel(ic.session)
err = msgmodel.Put(msg)
if err != nil {
log.Printf("Error from MessageModel.Put: %v", err)
// Set prompt if send fails
data.MsgBoxError = true
}
Expand All @@ -94,9 +95,5 @@ func (ic *IndexController) Post(w http.ResponseWriter, r *http.Request) {

func (ic *IndexController) render(w http.ResponseWriter, data RenderData) {
// Render page
err := utils.RenderTemplate(w, ic.config.PageContext, data, "index.tmpl", "elements/current_and_next.tmpl", "elements/banner.tmpl", "elements/message_box.tmpl", "elements/index_countdown.tmpl")
if err != nil {
log.Println(err)
return
}
utils.RenderTemplate(w, ic.config.PageContext, data, "index.tmpl", "elements/current_and_next.tmpl", "elements/banner.tmpl", "elements/message_box.tmpl", "elements/index_countdown.tmpl")
}
11 changes: 4 additions & 7 deletions controllers/not_found.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ package controllers

import (
"fmt"
"github.com/UniversityRadioYork/2016-site/models"
"github.com/UniversityRadioYork/myradio-go"
"log"
"net"
"net/http"

"github.com/UniversityRadioYork/2016-site/models"
"github.com/UniversityRadioYork/myradio-go"

"github.com/UniversityRadioYork/2016-site/structs"
"github.com/UniversityRadioYork/2016-site/utils"
)
Expand Down Expand Up @@ -57,9 +58,5 @@ func (sc *NotFoundController) Get(w http.ResponseWriter, r *http.Request) {
return
}
w.WriteHeader(404)
err := utils.RenderTemplate(w, sc.config.PageContext, nil, "404.tmpl")
if err != nil {
log.Println(err)
return
}
utils.RenderTemplate(w, sc.config.PageContext, nil, "404.tmpl")
}
14 changes: 3 additions & 11 deletions controllers/on_demand.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package controllers

import (
"log"
"net/http"

"github.com/UniversityRadioYork/2016-site/models"
Expand Down Expand Up @@ -29,8 +28,7 @@ func (onDemandC *OnDemandController) Get(w http.ResponseWriter, r *http.Request)
latestPodcasts, err := PodcastsM.GetAllPodcasts(10, 0)

if err != nil {
log.Println(err)
utils.RenderTemplate(w, onDemandC.config.PageContext, err, "404.tmpl")
onDemandC.handleError(w, r, err, "PodcastModel.GetAllPodcasts")
return
}

Expand All @@ -39,8 +37,7 @@ func (onDemandC *OnDemandController) Get(w http.ResponseWriter, r *http.Request)
latestTimeslots, err := OnDemandM.GetLastMixcloudTimeslots()

if err != nil {
log.Println(err)
utils.RenderTemplate(w, onDemandC.config.PageContext, err, "404.tmpl")
onDemandC.handleError(w, r, err, "OnDemandModel.GetLastMixcloudTimeslots")
return
}

Expand All @@ -52,10 +49,5 @@ func (onDemandC *OnDemandController) Get(w http.ResponseWriter, r *http.Request)
LatestTimeslots: latestTimeslots,
}

err = utils.RenderTemplate(w, onDemandC.config.PageContext, data, "on_demand.tmpl")
if err != nil {
log.Println(err)
return
}

utils.RenderTemplate(w, onDemandC.config.PageContext, data, "on_demand.tmpl")
}
20 changes: 7 additions & 13 deletions controllers/people.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package controllers

import (
"log"
"net/http"
"strconv"

"github.com/gorilla/mux"

"github.com/UniversityRadioYork/2016-site/models"
"github.com/UniversityRadioYork/2016-site/structs"
"github.com/UniversityRadioYork/2016-site/utils"
"github.com/UniversityRadioYork/myradio-go"
"github.com/gorilla/mux"
)

// PeopleController is the controller for the user bio page.
Expand All @@ -33,17 +33,16 @@ func (pc *PeopleController) Get(w http.ResponseWriter, r *http.Request) {
id, _ := strconv.Atoi(vars["id"])

user, officerships, credits, currentAndNext, err := pm.Get(id)
if err != nil {
pc.handleError(w, r, err, "PeopleModel.Get")
return
}

//404 when the user has no credits.
if len(credits) == 0 && len(officerships) == 0 {
utils.RenderTemplate(w, pc.config.PageContext, err, "404.tmpl")
return
}
if err != nil {
log.Println(err)
utils.RenderTemplate(w, pc.config.PageContext, err, "404.tmpl")
return
}

data := struct {
User *myradio.User
Expand All @@ -57,10 +56,5 @@ func (pc *PeopleController) Get(w http.ResponseWriter, r *http.Request) {
CurrentAndNext: currentAndNext,
}

err = utils.RenderTemplate(w, pc.config.PageContext, data, "people.tmpl", "elements/current_and_next.tmpl")
if err != nil {
log.Println(err)
return
}

utils.RenderTemplate(w, pc.config.PageContext, data, "people.tmpl", "elements/current_and_next.tmpl")
}
Loading
Loading