refactor: Replaces deprecated errors module with fmt #12682

MoritzThomasHuebner · 2022-05-11T01:45:59Z

refactoring: Replaced calls to errors.Wrap and errors.Wrapf with fmt.Errorf

Summary

This PR starts the process of removing the uses of github.com/pkg/errors which is no longer maintained. However, this change will remove stack traces in the output.

The issue was raised in #12632. The remaining uses of "github.com/pkg/errors" are removed in #12683. Splitting these PRs should make them easier to review.

This PR:

Replaces calls of the form errors.Wrap(err, wrap) with fmt.Errorf(wrap + ": %w", err) (or similar)
Replaces calls of the form errors.Wrapf(err, wrap, ...vars) with fmt.Errorf(wrap + ": %w", ...vars, err)

Go also automatically formatted some files while I was making these changes. There are a few white spaces that were added or removed unrelated to the change in error wrapping due to automatic formatting.

I attached an example below of the change in output. I obtained this by modifying the source code so a test would fail.

Old output:

    eap_aka_test.go:48: 
        	Error Trace:	eap_aka_test.go:48
        	Error:      	Received unexpected error:
        	            	I caused this error deliberately.
        	            	Error validating EAP packet
        	            	magma/cwf/gateway/services/uesim/servicers.(*UESimServer).HandleEap
        	            		/home/bmoritz/Documents/magma/cwf/gateway/services/uesim/servicers/eap.go:36
        	            	magma/cwf/gateway/services/uesim/servicers_test.TestEapAkaIdentityRequest
        	            		/home/bmoritz/Documents/magma/cwf/gateway/services/uesim/servicers/eap_aka_test.go:47
        	            	testing.tRunner
        	            		/usr/local/go/src/testing/testing.go:1439
        	            	runtime.goexit
        	            		/usr/local/go/src/runtime/asm_amd64.s:1571

New output

    eap_aka_test.go:48: 
        	Error Trace:	eap_aka_test.go:48
        	Error:      	Received unexpected error:
        	            	Error validating EAP packet: I caused this error deliberately.
        	Test:       	TestEapAkaIdentityRequest

Test Plan

Run unit tests.

Additional Information

This is my first PR!

This change is backwards-breaking

github-actions · 2022-05-11T01:46:12Z

Thanks for opening a PR! 💯

A couple initial guidelines

All commits must be signed off. This is enforced by DCO check.
All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

Howto

Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
Checks. All required CI checks must pass before merge.
Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

Developer Onboarding for onboarding as a new Magma developer
Development Workflow for guidance on your first pull request
CI Checks for points of contact for failing or flaky CI checks
GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

github-actions · 2022-05-11T06:19:13Z

feg-workflow

    2 files 202 suites 37s ⏱️
371 tests 371 ✔️ 0 💤 0 ❌
385 runs 385 ✔️ 0 💤 0 ❌

Results for commit 26c092f.

♻️ This comment has been updated with latest results.

github-actions · 2022-05-11T07:02:35Z

agw-workflow

    78 files   123 suites 6m 58s ⏱️
1 162 tests 1 152 ✔️ 9 💤 1 ❌
1 194 runs 1 184 ✔️ 9 💤 1 ❌

For more details on these failures, see this check.

Results for commit 26c092f.

♻️ This comment has been updated with latest results.

github-actions · 2022-05-11T07:54:47Z

cloud-workflow

    1 files   98 suites 1m 8s ⏱️
343 tests 322 ✔️ 0 💤 21 ❌

For more details on these failures, see this check.

Results for commit 26c092f.

♻️ This comment has been updated with latest results.

Neudrino · 2022-05-12T06:53:04Z

src/go/agwd/server/server.go

-			err,
-			"cleanupUnixSocket(logger=_, target.Endpoint=%s)",
-			path))
+		panic(fmt.Errorf("cleanupUnixSocket(logger=_, target.Endpoint=%s): %w", path, err))


While in general cleanup is nice, we might not want to change this in the same commit and PR, as the PR is already big enough. (Also lines below.)

This line and other instances of this in the file also change errors.Wrapf to fmt.Errorf. The reformatting was incidental.

Neudrino · 2022-05-12T06:54:34Z

cwf/gateway/integ_tests/test_runner.go

+func (tr *TestRunner) WaitForEnforcementStatsForRuleGreaterThanOrDoesNotExistFunc(imsi, ruleID string, min uint64) (*lteprotos.RuleRecord, bool) {
+	fmt.Printf("\tWaiting until %s, %s has more than %d bytes in enforcement stats or rule does not exist ...\n", imsi, ruleID, min)
+	records, err := tr.GetPolicyUsage()
+	if err != nil {
+		return nil, false
+	}
+	imsi = prependIMSIPrefix(imsi)
+	if records[imsi] == nil {
+		// Session is gone
+		fmt.Printf("\tSession for %s, does not exist...\n", imsi)
+		return nil, true
+	}
+	record := records[imsi][ruleID]
+	if record == nil {
+		// Session is gone
+		fmt.Printf("\tRule %s for %s, does not exist...\n", ruleID, imsi)
+		return nil, true
 	}
+	txBytes := record.BytesTx
+	if record.BytesTx < min {
+		return record, false
+	}
+	fmt.Printf("\t\u2713 %s, %s now passed %d > %d in enforcement stats!(%d%%)\n",
+		imsi, ruleID, txBytes, min, 100*txBytes/min)
+	return record, true
+}


This seems to be a white-space change only. While in general cleanup is nice, we might not want to change this in the same commit and PR, as the PR is already big enough. (Also lines below.)

I removed this white space change.

Neudrino · 2022-05-12T06:57:02Z

cwf/gateway/integ_tests/test_runner.go

 		newVolumeStr := fmt.Sprintf("%dK", newVolume/1000)

 		req.Volume = &wrappers.StringValue{Value: newVolumeStr}
 		fmt.Printf("- not enough traffic genereted, sending %dKB more. Will be around %d%% of volume requested\n",
 			newVolume/1000,
-			100 * (record.BytesTx+newVolume)/totalVolume,
+			100*(record.BytesTx+newVolume)/totalVolume,


Those changes of white-spaces around operators seem rather inconsistent and every PR seems to want to change them. I think in this PR, we should not touch them.

I removed this white space change.

Also added back in a nil check in `collectGarbageSQL` in `orc8r/clound/go/syncstore/store_writer.go` Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>

pull-request-size bot added the size/XXL Denotes a Pull Request that changes 1000+ lines. label May 11, 2022

github-actions bot added component: cwf component: feg FEG-gateway related issues component: orc8r Orchestrator-related issue labels May 11, 2022

MoritzThomasHuebner changed the title ~~refactoring: Replaced calls to errors.Wrap and errors.Wrapf with…~~ refactor: Replaced calls to errors.Wrap and errors.Wrapf with… May 11, 2022

MoritzThomasHuebner mentioned this pull request May 11, 2022

refactor: Remove more calls to pkg errors #12683

Closed

1 task

MoritzThomasHuebner force-pushed the Switch_from_github.com/pkg/errors_to_stdlib_errors branch from 9b15bdb to 84c5390 Compare May 11, 2022 07:38

wolfseb mentioned this pull request May 11, 2022

refactor(orc8r): Omit nil-checks for multierror.Append calls #12666

Merged

1 task

MoritzThomasHuebner force-pushed the Switch_from_github.com/pkg/errors_to_stdlib_errors branch 3 times, most recently from b093abc to f632b0c Compare May 12, 2022 06:39

MoritzThomasHuebner changed the title ~~refactor: Replaced calls to errors.Wrap and errors.Wrapf with…~~ refactor: Replaced calls to errors.Wrap and errors.Wrapf May 12, 2022

MoritzThomasHuebner force-pushed the Switch_from_github.com/pkg/errors_to_stdlib_errors branch from f632b0c to 8ea1e69 Compare May 12, 2022 06:44

magma deleted a comment from github-actions bot May 12, 2022

Neudrino reviewed May 12, 2022

View reviewed changes

MoritzThomasHuebner force-pushed the Switch_from_github.com/pkg/errors_to_stdlib_errors branch 3 times, most recently from 26c092f to 5e421be Compare May 16, 2022 00:19

refactor: Replaced calls to errors.Wrap and errors.Wrapf

ec7f48b

Also added back in a nil check in `collectGarbageSQL` in `orc8r/clound/go/syncstore/store_writer.go` Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>

MoritzThomasHuebner force-pushed the Switch_from_github.com/pkg/errors_to_stdlib_errors branch from 5e421be to ec7f48b Compare May 16, 2022 00:47

MoritzThomasHuebner changed the title ~~refactor: Replaced calls to errors.Wrap and errors.Wrapf~~ refactor: Replaces deprecated errors module with fmt May 16, 2022

MoritzThomasHuebner closed this May 16, 2022

MoritzThomasHuebner deleted the Switch_from_github.com/pkg/errors_to_stdlib_errors branch May 18, 2022 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Replaces deprecated errors module with fmt #12682

refactor: Replaces deprecated errors module with fmt #12682

MoritzThomasHuebner commented May 11, 2022 •

edited

github-actions bot commented May 11, 2022

github-actions bot commented May 11, 2022 •

edited

github-actions bot commented May 11, 2022 •

edited

github-actions bot commented May 11, 2022 •

edited

Neudrino May 12, 2022

MoritzThomasHuebner May 12, 2022

Neudrino May 12, 2022

MoritzThomasHuebner May 12, 2022

Neudrino May 12, 2022

MoritzThomasHuebner May 12, 2022

refactor: Replaces deprecated errors module with fmt #12682

refactor: Replaces deprecated errors module with fmt #12682

Conversation

MoritzThomasHuebner commented May 11, 2022 • edited

Summary

Test Plan

Additional Information

github-actions bot commented May 11, 2022

Howto

More info

github-actions bot commented May 11, 2022 • edited

feg-workflow

github-actions bot commented May 11, 2022 • edited

agw-workflow

github-actions bot commented May 11, 2022 • edited

cloud-workflow

Neudrino May 12, 2022

Choose a reason for hiding this comment

MoritzThomasHuebner May 12, 2022

Choose a reason for hiding this comment

Neudrino May 12, 2022

Choose a reason for hiding this comment

MoritzThomasHuebner May 12, 2022

Choose a reason for hiding this comment

Neudrino May 12, 2022

Choose a reason for hiding this comment

MoritzThomasHuebner May 12, 2022

Choose a reason for hiding this comment

MoritzThomasHuebner commented May 11, 2022 •

edited

github-actions bot commented May 11, 2022 •

edited

github-actions bot commented May 11, 2022 •

edited

github-actions bot commented May 11, 2022 •

edited