Skip to content

Conversation

@joybestourous
Copy link

@joybestourous joybestourous commented Dec 8, 2025

Modifies earlyAbortStreamHandler to include status details if present.

Most use cases of earlyAbortStreamHandler are for circumstances where there are certainly no error details (bad HTTP methods, bad content-type, internal error, etc). However, tap handlers also typically go through the earlyAbortStreamHandler. In http2_server.go:

	if t.inTapHandle != nil {
		var err error
		if s.ctx, err = t.inTapHandle(s.ctx, &tap.Info{FullMethodName: s.method, Header: mdata}); err != nil {
			t.mu.Unlock()
			if t.logger.V(logLevel) {
				t.logger.Infof("Aborting the stream early due to InTapHandle failure: %v", err)
			}
			stat, ok := status.FromError(err)
			if !ok {
				stat = status.New(codes.PermissionDenied, err.Error())
			}
			t.controlBuf.put(&earlyAbortStream{
				// ...
				status:         stat,  // <-- CAN have details!
			})

Yet the handler does not include error details by default, limiting how tap handlers can be used and breaking some user assumptions surrounding which information is propagated.
This PR fixes this by checking for status details and including the header for them if present.

RELEASE NOTES:

  • transport: status details from tap handler errors are now propagated

@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.21%. Comparing base (f9d2bdb) to head (1b5a905).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8754      +/-   ##
==========================================
- Coverage   83.35%   83.21%   -0.14%     
==========================================
  Files         418      418              
  Lines       32377    32389      +12     
==========================================
- Hits        26989    26954      -35     
- Misses       4022     4050      +28     
- Partials     1366     1385      +19     
Files with missing lines Coverage Δ
internal/transport/controlbuf.go 90.00% <100.00%> (+1.72%) ⬆️

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joybestourous joybestourous force-pushed the joy.bestourous/missing-details branch 3 times, most recently from 9bd760d to 10252fe Compare December 8, 2025 19:55
@joybestourous joybestourous force-pushed the joy.bestourous/missing-details branch from 10252fe to 1b5a905 Compare December 9, 2025 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant