Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Conversation

@ocket8888
Copy link
Contributor

This refactors api.Handler such that the return type is a single error instead of a user-facing error, a system-facing error, and a status code. It then handles the error specially if it is an api.Errors implementation. I also updated usage accordingly throughout the two endpoint handling packages that use api.Wrap.

Essentially this reduces

code, userErr, sysErr := someCall(inf)
if userErr != nil || sysErr != nil {
    return code, userErr, sysErr
}

to

err := someCall(inf)
if err != nil {
    return err
}

and

err := tx.QueryRow("My query", args...)
if err != nil {
    return http.StatusInternalServerError, nil, err
}

to

err := tx.QueryRow("My query", args...)
if err != nil {
    return err
}

Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

This is just a refactor; behavior should be unchanged and the existing tests should continue to pass.

PR submission checklist

  • This PR has tests
  • This PR has documentation
  • This PR doesn't need a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added Traffic Ops related to Traffic Ops low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution labels Jan 23, 2024
@codecov
Copy link

codecov bot commented Jan 23, 2024

Codecov Report

❌ Patch coverage is 19.81982% with 267 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.06%. Comparing base (3d933d0) to head (870c26a).
⚠️ Report is 145 commits behind head on master.

Files with missing lines Patch % Lines
traffic_ops/traffic_ops_golang/server/servers.go 7.95% 240 Missing and 3 partials ⚠️
...traffic_ops_golang/cdnfederation/cdnfederations.go 17.24% 24 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #7926      +/-   ##
============================================
+ Coverage     29.09%   32.06%   +2.97%     
  Complexity       98       98              
============================================
  Files           605      722     +117     
  Lines         77535    82925    +5390     
  Branches         90      970     +880     
============================================
+ Hits          22558    26590    +4032     
- Misses        52894    54183    +1289     
- Partials       2083     2152      +69     
Flag Coverage Δ
golib_unit 53.85% <ø> (+<0.01%) ⬆️
grove_unit 12.02% <ø> (ø)
t3c_unit 5.88% <ø> (ø)
traffic_monitor_unit 25.47% <ø> (ø)
traffic_ops_unit 22.15% <19.81%> (+0.06%) ⬆️
traffic_portal_v2 74.37% <ø> (?)
traffic_stats_unit 10.78% <ø> (ø)
unit_tests 29.41% <19.81%> (+3.46%) ⬆️

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.

@ocket8888 ocket8888 force-pushed the to/api-wrap-refactor branch from d5e3019 to 870c26a Compare January 25, 2024 23:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant