Add stack trace logging for internal server errors (500) #2204

aliraza556 · 2024-12-16T20:27:48Z

Describe your changes

Issue ticket number and link: #2186

Closed: #2186

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist before requesting a review

I have performed a self-review of my code
I have tested on Chrome and Firefox
I have tested on a mobile device
I have provided a screenshot or recording of changes in my PR if there were updates to the frontend

aliraza556 · 2024-12-16T20:34:49Z

Hi @kevkevinpal, Please review this PR.

kevkevinpal · 2024-12-16T22:06:24Z

routes/index.go

@@ -145,17 +147,27 @@ func getFromAuth(path string) (*extractResponse, error) {
 // Middleware to handle InternalServerError
 func internalServerErrorHandler(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Println("Inside Internal Server Middleware")


can you remove this comment it would unnecessarily flood the logs for every request

@kevkevinpal Sure

@kevkevinpal Done.

kevkevinpal · 2024-12-16T22:28:59Z

routes/test_routes.go

+	r := chi.NewRouter()
+
+	r.Get("/internal-server-error", func(w http.ResponseWriter, r *http.Request) {
+		http.Error(w, "Forced internal server error", http.StatusInternalServerError)


instead of sending back http.Error can you try doing a try: except but forcing the exception

right now when I look at the stacktrace I expect it to come from this test_routes.go file but its grabbing the stacktrace from in the middleware

this is the output I got

Internal Server Error: GET /test/internal-server-error Stack Trace: goroutine 53 [running]: github.com/stakwork/sphinx-tribes/routes.internalServerErrorHandler.func1({0x7f6acd306f30, 0xc000394fc0}, 0xc000371440) /mnt/shared_drive/DEVDIR/sphinx-tribes/routes/index.go:157 +0x187 net/http.HandlerFunc.ServeHTTP(0x413ea5?, {0x7f6acd306f30?, 0xc000394fc0?}, 0x2239a01?) /usr/local/go/src/net/http/server.go:2171 +0x29 github.com/go-chi/chi/middleware.Recoverer.func1({0x7f6acd306f30?, 0xc000394fc0?}, 0xc00061bce0?) /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/middleware/recoverer.go:37 +0x78 net/http.HandlerFunc.ServeHTTP(0x22556e0?, {0x7f6acd306f30?, 0xc000394fc0?}, 0x18?) /usr/local/go/src/net/http/server.go:2171 +0x29 github.com/go-chi/chi/middleware.init.0.RequestLogger.func1.1({0x18b0e70, 0xc0002ba8c0}, 0xc000370ea0) /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/middleware/logger.go:57 +0x16a net/http.HandlerFunc.ServeHTTP(0x18b2f30?, {0x18b0e70?, 0xc0002ba8c0?}, 0x1897ce0?) /usr/local/go/src/net/http/server.go:2171 +0x29 github.com/go-chi/chi/middleware. 2024/12/16 17:23:43 [fedora/NwyCgyx1sr-000003] "GET http://100.97.107.124:5002/test/internal-server-error HTTP/1.1" from 100.89.29.16:59100 - 500 51B in 250.927µs

aliraza556 · 2024-12-17T07:45:09Z

Hi @kevkevinpal, please review this PR. If anything is needed, please let me know.

kevkevinpal · 2024-12-17T14:30:37Z

The new commits look good!

This is the new error I see now

Internal Server Error: GET /test/internal-server-error
Error: Forced internal server error
Stack Trace:
goroutine 67 [running]:
github.com/stakwork/sphinx-tribes/routes.internalServerErrorHandler.func1.1()
        /mnt/shared_drive/DEVDIR/sphinx-tribes/routes/index.go:156 +0x85
panic({0x134b4e0?, 0x18997f0?})
        /usr/local/go/src/runtime/panic.go:770 +0x132
github.com/stakwork/sphinx-tribes/routes.TestRoutes.func1({0xc000882b70?, 0x15e38eb?}, 0xc0004ee790?)
        /mnt/shared_drive/DEVDIR/sphinx-tribes/routes/test_routes.go:13 +0x25
net/http.HandlerFunc.ServeHTTP(0x13c25a0?, {0x7efcadf4e368?, 0xc000138200?}, 0xc00059ce28?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/go-chi/chi.(*Mux).routeHTTP(0xc000679080, {0x7efcadf4e368, 0xc000138200}, 0xc000340900)
        /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/mux.go:436 +0x1f2
net/http.HandlerFunc.ServeHTTP(0xc00015f5c8?, {0x7efcadf4e368?, 0xc000138200?}, 0xc00015f5a8?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/go-chi/chi.(*Mux).ServeHTTP(0xc000679080, {0x7efcadf4e368, 0xc000138200}, 0xc000340900)
        /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/mux.go:70 +0x331
github.com/go-chi/chi.(*Mux).Mount.func1({0x7efcadf4e368, 0xc000138200}, 0xc000340900)
        /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/mux.go:311 +0x1c2
net/http.HandlerFunc.ServeHTTP(0x13c25a0?, {0x7efcadf4e368?, 0xc000138200?}, 0xc000046484?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/go-chi/chi.(*Mux).routeHTTP(0xc0006780c0, {0x7efcadf4e368, 0xc000138200}, 0xc000340900)
        /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/mux.go:436 +0x1f2
net/http.HandlerFunc.ServeHTTP(0xc0003407e0?, {0x7efcadf4e368?, 0xc000138200?}, 0xc00014f768?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/stakwork/sphinx-tribes/routes.initChi.Timeout.func1.1({0x7efcadf4e368, 0xc000138200}, 0xc0003407e0)
        /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/middleware/timeout.go:45 +0xfa
net/http.HandlerFunc.ServeHTTP(0xc0002a6000?, {0x7efcadf4e368?, 0xc000138200?}, 0xc0003407e0?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/rs/cors.(*Cors).Handler-fm.(*Cors).Handler.func1({0x7efcadf4e368, 0xc000138200}, 0xc0003407e0)
        /home/kevkevin/go/pkg/mod/github.com/rs/[email protected]/cors.go:281 +0x184
net/http.HandlerFunc.ServeHTTP(0x223f320?, {0x7efcadf4e368?, 0xc000138200?}, 0xc00014f8c8?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/stakwork/sphinx-tribes/routes.internalServerErrorHandler.func1({0x7efcadf4e318, 0xc0001381c0}, 0xc0003407e0)
        /mnt/shared_drive/DEVDIR/sphinx-tribes/routes/index.go:170 +0x169
net/http.HandlerFunc.ServeHTTP(0x413ea5?, {0x7efcadf4e318?, 0xc0001381c0?}, 0x2239a01?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/go-chi/chi/middleware.Recoverer.func1({0x7efcadf4e318?, 0xc0001381c0?}, 0xc000652030?)
        /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/middleware/recoverer.go:37 +0x78
net/http.HandlerFunc.ServeHTTP(0x22556e0?, {0x7efcadf4e318?, 0xc0001381c0?}, 0x18?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/go-chi/chi/middleware.init.0.RequestLogger.func1.1({0x18b0e90, 0xc0008882a0}, 0xc0003406c0)
        /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/middleware/logger.go:57 +0x16a
net/http.HandlerFunc.ServeHTTP(0x18b2f50?, {0x18b0e90?, 0xc0008882a0?}, 0x1897cf0?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/go-chi/chi/middleware.RequestID.func1({0x18b0e90, 0xc0008882a0}, 0xc0003405a0)
        /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/middleware/request_id.go:76 +0x20e
net/http.HandlerFunc.ServeHTTP(0x18b2f88?, {0x18b0e90?, 0xc0008882a0?}, 0x22396f0?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
github.com/go-chi/chi.(*Mux).ServeHTTP(0xc0006780c0, {0x18b0e90, 0xc0008882a0}, 0xc0003dc7e0)
        /home/kevkevin/go/pkg/mod/github.com/go-chi/[email protected]/mux.go:87 +0x2ef
net/http.serverHandler.ServeHTTP({0xc000394570?}, {0x18b0e90?, 0xc0008882a0?}, 0x6?)
        /usr/local/go/src/net/http/server.go:3142 +0x8e
net/http.(*conn).serve(0xc00018a1b0, {0x18b2f50, 0xc0007a82d0})
        /usr/local/go/src/net/http/server.go:2044 +0x5e8
created by net/http.(*Server).Serve in goroutine 57
        /usr/local/go/sr

I will create a new issue to replace the places where we just throw internal server error and instead use a panic instead

grep -nri "http\.error.*StatusInternalServerError" ./
./auth/auth.go:160:                     http.Error(w, http.StatusText(500), http.StatusInternalServerError)
./handlers/features.go:565:             http.Error(w, "API key not set in environment", http.StatusInternalServerError)
./handlers/features.go:583:             http.Error(w, "Failed to encode payload", http.StatusInternalServerError)
./handlers/features.go:589:             http.Error(w, "Failed to create request to Stakwork API", http.StatusInternalServerError)
./handlers/features.go:598:             http.Error(w, "Failed to send request to Stakwork API", http.StatusInternalServerError)
./handlers/features.go:605:             http.Error(w, "Failed to read response from Stakwork API", http.StatusInternalServerError)
./handlers/features.go:639:             http.Error(w, "HOST environment variable not set", http.StatusInternalServerError)
./handlers/features.go:653:             http.Error(w, "API key not set in environment", http.StatusInternalServerError)
./handlers/features.go:671:             http.Error(w, "Failed to encode payload", http.StatusInternalServerError)
./handlers/features.go:677:             http.Error(w, "Failed to create request to Stakwork API", http.StatusInternalServerError)
./handlers/features.go:686:             http.Error(w, "Failed to send request to Stakwork API", http.StatusInternalServerError)
./handlers/features.go:693:             http.Error(w, "Failed to read response from Stakwork API", http.StatusInternalServerError)
./handlers/meme.go:46:          http.Error(w, "Unable to create file", http.StatusInternalServerError)
./handlers/meme.go:54:          http.Error(w, "Unable to copy saved file", http.StatusInternalServerError)
./handlers/workflow.go:44:              http.Error(w, "Failed to process workflow request", http.StatusInternalServerError)
./handlers/workflow.go:52:              http.Error(w, "Failed to create workflow request", http.StatusInternalServerError)
./handlers/workflow.go:87:              http.Error(w, "Failed to retrieve original request", http.StatusInternalServerError)
./handlers/workflow.go:97:              http.Error(w, "Failed to check processing requirements", http.StatusInternalServerError)
./handlers/workflow.go:112:             http.Error(w, "Failed to update workflow request", http.StatusInternalServerError)
./routes/index.go:166:                          http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)

feat: add stack trace logging for 500 internal server errors

af75b81

kevkevinpal reviewed Dec 16, 2024

View reviewed changes

remove unnecessarily comment

7c0cefa

aliraza556 requested a review from kevkevinpal December 16, 2024 22:11

kevkevinpal reviewed Dec 16, 2024

View reviewed changes

aliraza556 added 2 commits December 17, 2024 12:37

feat: Implement stack trace capture for 500 errors

b4887c4

feat: Implement stack trace capture for 500 errors

0711648

aliraza556 requested a review from kevkevinpal December 17, 2024 07:43

kevkevinpal merged commit e5fcf13 into stakwork:master Dec 17, 2024
2 checks passed

kevkevinpal mentioned this pull request Dec 17, 2024

Replace where we return with 500 to raise an error instead #2212

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add stack trace logging for internal server errors (500) #2204

Add stack trace logging for internal server errors (500) #2204

aliraza556 commented Dec 16, 2024

aliraza556 commented Dec 16, 2024

kevkevinpal Dec 16, 2024

aliraza556 Dec 16, 2024

aliraza556 Dec 16, 2024

kevkevinpal Dec 16, 2024

aliraza556 commented Dec 17, 2024

kevkevinpal commented Dec 17, 2024

Add stack trace logging for internal server errors (500) #2204

Add stack trace logging for internal server errors (500) #2204

Conversation

aliraza556 commented Dec 16, 2024

Describe your changes

Issue ticket number and link: #2186

Closed: #2186

Type of change

Checklist before requesting a review

aliraza556 commented Dec 16, 2024

kevkevinpal Dec 16, 2024

Choose a reason for hiding this comment

aliraza556 Dec 16, 2024

Choose a reason for hiding this comment

aliraza556 Dec 16, 2024

Choose a reason for hiding this comment

kevkevinpal Dec 16, 2024

Choose a reason for hiding this comment

aliraza556 commented Dec 17, 2024

kevkevinpal commented Dec 17, 2024

This is the new error I see now

I will create a new issue to replace the places where we just throw internal server error and instead use a panic instead