distinguish user errors from internal errors by aaryan359 · Pull Request #7980 · jaegertracing/jaeger

aaryan359 · 2026-02-04T06:11:48Z

Which problem is this PR solving?

Resolves [Bug]: Frontend service returns 500 for invalid customer ID instead of 400 #7979

Description of the changes

Distinguish user-caused errors from internal server errors in the HotROD example.
Return appropriate HTTP status codes (e.g. 400 Bad Request) for invalid input instead of 500 Internal Server Error.
Improve error handling to align with expected REST API behavior and existing TODO comments in the code.

How was this change tested?

Built and ran the HotROD example locally.
Sent requests with invalid customer IDs using curl and verified the service returns a clear error message with the correct HTTP status code.
Confirmed existing functionality remains unchanged for valid requests.

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
I have signed all commits
I have added unit tests for the new functionality
I have run lint and test steps successfully
- for jaeger: make lint test

codecov · 2026-02-04T06:19:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.48%. Comparing base (9c047cb) to head (b2ce304).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7980      +/-   ##
==========================================
+ Coverage   95.47%   95.48%   +0.01%     
==========================================
  Files         316      316              
  Lines       16756    16756              
==========================================
+ Hits        15997    15999       +2     
+ Misses        593      592       -1     
+ Partials      166      165       -1

Flag	Coverage Δ
badger_v1	`9.16% <ø> (ø)`
badger_v2	`1.89% <ø> (ø)`
cassandra-4.x-v1-manual	`13.35% <ø> (ø)`
cassandra-4.x-v2-auto	`1.88% <ø> (ø)`
cassandra-4.x-v2-manual	`1.88% <ø> (ø)`
cassandra-5.x-v1-manual	`13.35% <ø> (ø)`
cassandra-5.x-v2-auto	`1.88% <ø> (ø)`
cassandra-5.x-v2-manual	`1.88% <ø> (ø)`
clickhouse	`1.97% <ø> (ø)`
elasticsearch-6.x-v1	`17.19% <ø> (ø)`
elasticsearch-7.x-v1	`17.22% <ø> (ø)`
elasticsearch-8.x-v1	`17.37% <ø> (ø)`
elasticsearch-8.x-v2	`1.89% <ø> (ø)`
elasticsearch-9.x-v2	`1.89% <ø> (ø)`
grpc_v1	`8.40% <ø> (ø)`
grpc_v2	`1.89% <ø> (ø)`
kafka-3.x-v2	`1.89% <ø> (ø)`
memory_v2	`1.89% <ø> (ø)`
opensearch-1.x-v1	`17.26% <ø> (ø)`
opensearch-2.x-v1	`17.26% <ø> (ø)`
opensearch-2.x-v2	`1.89% <ø> (ø)`
opensearch-3.x-v2	`1.89% <ø> (ø)`
query	`1.89% <ø> (ø)`
tailsampling-processor	`0.54% <ø> (ø)`
unittests	`94.16% <ø> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

aaryan359 · 2026-02-07T11:44:30Z

@jkowall and @yurishkuro, can you review this please? Does this look good?

Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>

jkowall · 2026-02-08T04:13:08Z

Seems like some issue with the tests, please verify and fix. Copilot is reviewing it now too. Thanks.

Copilot

Pull request overview

Updates the HotROD example’s error handling so that user-caused failures (e.g., invalid customer IDs) are surfaced as client errors (400-level) instead of being treated as internal server failures (500).

Changes:

Introduces typed HTTP-related errors (BadRequestError, HTTPError) and an auto-mapping helper (HandleErrorAuto) in httperr.
Updates frontend and customer services to use HandleErrorAuto when handling downstream/service errors.
Adds unit tests covering the new error types/handler behavior and the customer database invalid-customer path.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
examples/hotrod/services/frontend/server.go	Uses auto HTTP status mapping when `bestETA.Get()` fails to avoid always returning 500.
examples/hotrod/services/customer/server.go	Uses auto HTTP status mapping for database lookup errors.
examples/hotrod/services/customer/database.go	Returns a typed bad-request error for invalid customer IDs.
examples/hotrod/pkg/tracing/http.go	Propagates downstream HTTP status codes via a typed `HTTPError`.
examples/hotrod/pkg/httperr/httperr.go	Adds typed errors and `HandleErrorAuto` to map errors to appropriate status codes.
examples/hotrod/pkg/httperr/httperr_test.go	Adds tests for `HandleError`, `HandleErrorAuto`, and the new error constructors.
examples/hotrod/services/customer/database_test.go	Adds tests validating success cases and the invalid-customer error type.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-08T04:21:51Z

examples/hotrod/pkg/httperr/httperr.go

@@ -17,3 +49,27 @@ func HandleError(w http.ResponseWriter, err error, statusCode int) bool {
 	http.Error(w, string(err.Error()), statusCode)


Redundant conversion: err.Error() already returns a string, so wrapping it in string(...) is unnecessary and may be flagged by linters (e.g., staticcheck S1025).

Suggested change

http.Error(w, string(err.Error()), statusCode)

http.Error(w, err.Error(), statusCode)

Copilot · 2026-02-08T04:21:52Z

examples/hotrod/pkg/httperr/httperr.go

 	"net/http"
 )

+// BadRequestError represents a user error (4xx status code)


The comment says BadRequestError represents a user error "(4xx status code)", but the implementation only maps this type to HTTP 400. Consider updating the comment to avoid implying it can represent other 4xx codes.

Suggested change

// BadRequestError represents a user error (4xx status code)

// BadRequestError represents a user error mapped to HTTP 400 (Bad Request)

Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>

aaryan359 · 2026-02-08T07:20:19Z

@jkowall, I fixed the test and also improved the code review. Now is it looking good.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T15:37:20Z

examples/hotrod/services/frontend/server.go

+	if httperr.HandleErrorAuto(w, err) {
 		s.logger.For(ctx).Error("request failed", zap.Error(err))
 		return


HandleErrorAuto may be returning a 4xx (e.g., invalid customer ID), but this branch always logs at Error level. That can create noisy logs/alerts for user-caused failures. Consider downgrading the log level for client errors (e.g., BadRequestError or HTTPError with StatusCode < 500) and reserving Error logs for 5xx/unexpected errors.

Copilot · 2026-02-11T15:37:21Z

examples/hotrod/services/customer/server.go

+	if httperr.HandleErrorAuto(w, err) {
 		s.logger.For(ctx).Error("request failed", zap.Error(err))
 		return


Same as frontend: this logs at Error level even when HandleErrorAuto maps the error to a 4xx user error. Consider logging 4xx as Info/Debug and using Error only for 5xx, based on BadRequestError / HTTPError.StatusCode.

Copilot · 2026-02-11T15:37:21Z

examples/hotrod/services/customer/database_test.go

+func TestDatabaseGetValidCustomer(t *testing.T) {
+	logger := log.NewFactory(zap.NewNop())
+	db := newDatabase(trace.NewNoopTracerProvider().Tracer("test"), logger)
+
+	// Test getting a valid customer
+	customer, err := db.Get(context.Background(), 123)
+	require.NoError(t, err)


These unit tests call db.Get, which sleeps using config.MySQLGetDelay (default is 300ms) and can make the test suite noticeably slower. Consider overriding config.MySQLGetDelay/MySQLGetDelayStdDev (and optionally MySQLMutexDisabled) to near-zero in the tests (with t.Cleanup to restore) so tests stay fast.

Copilot · 2026-02-11T15:37:22Z

examples/hotrod/pkg/tracing/http.go

 	if res.StatusCode >= http.StatusBadRequest {
 		body, err := io.ReadAll(res.Body)
 		if err != nil {
 			return err
 		}
-		return errors.New(string(body))
+		return httperr.NewHTTPError(res.StatusCode, string(body))
 	}


The response body from http.Error typically already ends with a newline; wrapping it directly into an HTTPError and later re-emitting it via http.Error can lead to double newlines in the propagated message. Consider trimming whitespace (e.g., strings.TrimSpace) when constructing the HTTPError message.

aaryan359 requested a review from a team as a code owner February 4, 2026 06:11

dosubot bot added the enhancement label Feb 4, 2026

aaryan359 added 5 commits February 7, 2026 22:04

distinguish user errors from internal errors

fc07005

Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>

chore: rerun CI

881b783

Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>

chore: rerun CI

d3903fb

Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>

chore: rerun CI

f71396c

Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>

chore: fix lint CI

2c29ceb

Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>

aaryan359 force-pushed the fix/frontend-error-handling branch from 0fa7435 to 2c29ceb Compare February 7, 2026 16:35

jkowall requested a review from Copilot February 8, 2026 04:12

jkowall added the changelog:bugfix-or-minor-feature label Feb 8, 2026

Copilot started reviewing on behalf of jkowall February 8, 2026 04:12 View session

github-actions bot added the waiting-for-author PR is waiting for author to respond to maintainer's comments label Feb 8, 2026

Copilot AI reviewed Feb 8, 2026

View reviewed changes

fix copilot changes

1e46e13

Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>

github-actions bot removed the waiting-for-author PR is waiting for author to respond to maintainer's comments label Feb 8, 2026

Merge branch 'main' into fix/frontend-error-handling

b2ce304

Copilot AI review requested due to automatic review settings February 11, 2026 15:20

Copilot started reviewing on behalf of aaryan359 February 11, 2026 15:24 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

distinguish user errors from internal errors#7980

distinguish user errors from internal errors#7980
aaryan359 wants to merge 7 commits intojaegertracing:mainfrom
aaryan359:fix/frontend-error-handling

aaryan359 commented Feb 4, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 4, 2026 •

edited

Loading

Uh oh!

aaryan359 commented Feb 7, 2026 •

edited

Loading

Uh oh!

jkowall commented Feb 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

aaryan359 commented Feb 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -17,3 +49,27 @@ func HandleError(w http.ResponseWriter, err error, statusCode int) bool {
		http.Error(w, string(err.Error()), statusCode)

	http.Error(w, string(err.Error()), statusCode)
	http.Error(w, err.Error(), statusCode)

	// BadRequestError represents a user error (4xx status code)
	// BadRequestError represents a user error mapped to HTTP 400 (Bad Request)

Comments

Conversation

aaryan359 commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

Uh oh!

codecov bot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aaryan359 commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkowall commented Feb 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

aaryan359 commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aaryan359 commented Feb 4, 2026 •

edited

Loading

codecov bot commented Feb 4, 2026 •

edited

Loading

aaryan359 commented Feb 7, 2026 •

edited

Loading

aaryan359 commented Feb 8, 2026 •

edited

Loading