Adding stack to errors #1413

wazsone · 2023-01-24T10:47:33Z

Description

Make sure that all errors have a stack and message

Before:

Running tool: /usr/local/go/bin/go test -timeout 30s -run ^TestExcludedPrefixesClient_Close_PrefixesAreRemoved$ github.com/networkservicemesh/sdk/pkg/networkservice/common/excludedprefixes

2023/02/01 06:58:13 [ERROR] [ExcludedPrefixesClient:Request] Source or destination IPs are overlapping with excluded prefixes, srcIPs: [a.16.0.100/32], dstIPs: [b.16.0.103/32], excluded prefixes: [], error: invalid CIDR address: a.16.0.100/32
--- FAIL: TestExcludedPrefixesClient_Close_PrefixesAreRemoved (0.00s)
    /home/ubuntu/nsm/wazsone/sdk/pkg/networkservice/common/excludedprefixes/client_test.go:153: 
        	Error Trace:	/home/ubuntu/nsm/wazsone/sdk/pkg/networkservice/common/excludedprefixes/client_test.go:153
        	Error:      	Received unexpected error:
        	            	invalid CIDR address: a.16.0.100/32
        	Test:       	TestExcludedPrefixesClient_Close_PrefixesAreRemoved
FAIL
FAIL	github.com/networkservicemesh/sdk/pkg/networkservice/common/excludedprefixes	0.016s
FAIL

After:

Running tool: /usr/local/go/bin/go test -timeout 30s -run ^TestExcludedPrefixesClient_Close_PrefixesAreRemoved$ github.com/networkservicemesh/sdk/pkg/networkservice/common/excludedprefixes

2023/02/01 06:59:29 [ERROR] [ExcludedPrefixesClient:Request] Source or destination IPs are overlapping with excluded prefixes, srcIPs: [a.16.0.100/32], dstIPs: [b.16.0.103/32], excluded prefixes: [], error: failed to parse a.16.0.100/32 as CIDR: invalid CIDR address: a.16.0.100/32
--- FAIL: TestExcludedPrefixesClient_Close_PrefixesAreRemoved (0.00s)
    /home/ubuntu/nsm/wazsone/sdk/pkg/networkservice/common/excludedprefixes/client_test.go:153: 
        	Error Trace:	/home/ubuntu/nsm/wazsone/sdk/pkg/networkservice/common/excludedprefixes/client_test.go:153
        	Error:      	Received unexpected error:
        	            	invalid CIDR address: a.16.0.100/32
        	            	failed to parse a.16.0.100/32 as CIDR
        	            	github.com/networkservicemesh/sdk/pkg/networkservice/common/excludedprefixes.validateIPs
        	            		/home/ubuntu/nsm/wazsone/sdk/pkg/networkservice/common/excludedprefixes/client.go:230
        	            	github.com/networkservicemesh/sdk/pkg/networkservice/common/excludedprefixes.(*excludedPrefixesClient).Request
        	            		/home/ubuntu/nsm/wazsone/sdk/pkg/networkservice/common/excludedprefixes/client.go:123
        	            	github.com/networkservicemesh/sdk/pkg/networkservice/common/excludedprefixes_test.TestExcludedPrefixesClient_Close_PrefixesAreRemoved
        	            		/home/ubuntu/nsm/wazsone/sdk/pkg/networkservice/common/excludedprefixes/client_test.go:152
        	            	testing.tRunner
        	            		/usr/local/go/src/testing/testing.go:1439
        	            	runtime.goexit
        	            		/usr/local/go/src/runtime/asm_amd64.s:1571
        	Test:       	TestExcludedPrefixesClient_Close_PrefixesAreRemoved
FAIL
FAIL	github.com/networkservicemesh/sdk/pkg/networkservice/common/excludedprefixes	0.014s
FAIL

Issue link

#705

How Has This Been Tested?

Added unit testing to cover
Tested manually
Tested by integration testing
Have not tested

Types of changes

codecov-commenter · 2023-01-26T10:56:49Z

Codecov Report

❗ No coverage uploaded for pull request base (main@f45a84e). Click here to learn what that means.
Patch has no changes to coverable lines.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1413   +/-   ##
=======================================
  Coverage        ?   67.60%           
=======================================
  Files           ?      267           
  Lines           ?    11625           
  Branches        ?        0           
=======================================
  Hits            ?     7859           
  Misses          ?     3257           
  Partials        ?      509

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru>

glazychev-art

In general it looks good to me.
Left a couple of comments

glazychev-art · 2023-02-02T05:12:12Z

pkg/networkservice/utils/checks/checkconnection/client.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2020-2022 Cisco and/or its affiliates.
+// Copyright (c) 2020-2023 Cisco and/or its affiliates.


unnecessary change

glazychev-art · 2023-02-02T05:28:57Z

pkg/networkservice/core/eventchannel/monitor_connection_client.go

@@ -47,7 +47,7 @@ func NewMonitorConnectionMonitorConnectionsClient(ctx context.Context, eventCh <
 func (m *monitorConnectionMonitorConnectionsClient) Recv() (*networkservice.ConnectionEvent, error) {
 	select {
 	case <-m.ctx.Done():
-		return nil, m.ctx.Err()
+		return nil, errors.WithStack(m.ctx.Err())


I think it should be "application context is done"
(and other places like m.ctx.Err())

Thank you, done

Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru>

…k@main PR link: networkservicemesh/sdk#1413 Commit: 3aa9aa0 Author: Ruslan Bayandinov Date: 2023-02-13 19:45:19 +0600 Message: - Adding stack to errors (#1413) * adding stack to errors Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru> * update generated files Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru> * update errors handling in pkg/ipam Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru> * update errors handling in pkg/networkservice Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru> * update errors handling in pkg/registry Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru> * update errors handling in pkg/tools Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru> * update context errors handling Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru> * address review comments Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru> --------- Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>

wazsone force-pushed the feature/adding-errors-stack branch 6 times, most recently from 70ff94a to ef0c48e Compare January 25, 2023 03:35

wazsone self-assigned this Jan 25, 2023

wazsone changed the title ~~Adding stack to errors~~ Draft: Adding stack to errors Jan 25, 2023

wazsone marked this pull request as draft January 25, 2023 07:45

wazsone force-pushed the feature/adding-errors-stack branch from ef0c48e to 65a85f5 Compare January 26, 2023 10:07

wazsone force-pushed the feature/adding-errors-stack branch 2 times, most recently from 17770b7 to 0f5a54f Compare January 26, 2023 11:02

wazsone marked this pull request as ready for review January 26, 2023 11:09

wazsone changed the title ~~Draft: Adding stack to errors~~ Adding stack to errors Jan 26, 2023

wazsone added 2 commits January 31, 2023 04:45

adding stack to errors

fc3717c

Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru>

update generated files

2cc4789

Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru>

wazsone force-pushed the feature/adding-errors-stack branch from 0f5a54f to 4f8be35 Compare January 31, 2023 04:46

wazsone added 2 commits January 31, 2023 17:43

update errors handling in pkg/ipam

7c60590

Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru>

update errors handling in pkg/networkservice

2d18192

Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru>

wazsone force-pushed the feature/adding-errors-stack branch from 4f8be35 to 2d18192 Compare February 1, 2023 03:40

update errors handling in pkg/registry

a00e6a2

Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru>

wazsone requested review from denis-tingaikin and glazychev-art February 1, 2023 07:07

update errors handling in pkg/tools

b6301b4

Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru>

wazsone force-pushed the feature/adding-errors-stack branch from 04f2e44 to b6301b4 Compare February 1, 2023 07:21

glazychev-art reviewed Feb 2, 2023

View reviewed changes

wazsone force-pushed the feature/adding-errors-stack branch 2 times, most recently from ea64870 to ea1b01f Compare February 2, 2023 11:45

update context errors handling

44fa93d

Signed-off-by: Ruslan Bayandinov <wazsone@ya.ru>

wazsone requested a review from denis-tingaikin February 13, 2023 13:32

denis-tingaikin approved these changes Feb 13, 2023

View reviewed changes

denis-tingaikin merged commit 3aa9aa0 into networkservicemesh:main Feb 13, 2023

nsmbot mentioned this pull request Feb 13, 2023

Update from update/networkservicemesh/sdk networkservicemesh/cmd-cluster-info-k8s#52

Merged

nsmbot mentioned this pull request Feb 13, 2023

Update from update/networkservicemesh/sdk networkservicemesh/cmd-map-ip-k8s#130

Merged

nsmbot mentioned this pull request Feb 13, 2023

Update from update/networkservicemesh/sdk networkservicemesh/cmd-admission-webhook-k8s#251

Merged

This was referenced Feb 13, 2023

Update from update/networkservicemesh/sdk networkservicemesh/cmd-nsc-init#593

Merged

Update from update/networkservicemesh/sdk networkservicemesh/cmd-nse-remote-vlan#142

Merged

This was referenced Feb 13, 2023

Update from update/networkservicemesh/sdk networkservicemesh/cmd-ipam-vl3#113

Merged

Update from update/networkservicemesh/sdk networkservicemesh/cmd-nsmgr#582

Merged

glazychev-art mentioned this pull request Aug 1, 2023

[qfix]: Fix monitorFilter #1498

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding stack to errors #1413

Adding stack to errors #1413

wazsone commented Jan 24, 2023 •

edited

Loading

codecov-commenter commented Jan 26, 2023 •

edited

Loading

glazychev-art left a comment

glazychev-art Feb 2, 2023

wazsone Feb 2, 2023

glazychev-art Feb 2, 2023

wazsone Feb 2, 2023

		@@ -1,4 +1,4 @@
		// Copyright (c) 2020-2022 Cisco and/or its affiliates.
		// Copyright (c) 2020-2023 Cisco and/or its affiliates.

Adding stack to errors #1413

Adding stack to errors #1413

Conversation

wazsone commented Jan 24, 2023 • edited Loading

Description

Issue link

How Has This Been Tested?

Types of changes

codecov-commenter commented Jan 26, 2023 • edited Loading

Codecov Report

glazychev-art left a comment

Choose a reason for hiding this comment

glazychev-art Feb 2, 2023

Choose a reason for hiding this comment

wazsone Feb 2, 2023

Choose a reason for hiding this comment

glazychev-art Feb 2, 2023

Choose a reason for hiding this comment

wazsone Feb 2, 2023

Choose a reason for hiding this comment

wazsone commented Jan 24, 2023 •

edited

Loading

codecov-commenter commented Jan 26, 2023 •

edited

Loading