Mention domain in the error message in NNS #88

elebedeva · 2024-04-16T11:28:38Z

elebedeva commented

2024-04-16 11:28:38 +00:00

Close #81

Fixed error message looks like this:

❯ ./bin/frostfs-cli container create -r 127.0.0.1:8080 --wallet ./dev/storage/wallet01.json --await --policy 'REP 1' --nns-name kapusta --nns-zone makaroni.ns --await
Enter password > 
put container rpc error: status: code = 1024 message = frostfs error: chain/client: contract execution finished with state FAULT; exception: at instruction 1274 (THROW): unhandled exception: "domain does not exist or is expired: makaroni.ns"

Signed-off-by: Ekaterina Lebedeva ekaterina.lebedeva@yadro.com

Close #81 Fixed error message looks like this: ``` ❯ ./bin/frostfs-cli container create -r 127.0.0.1:8080 --wallet ./dev/storage/wallet01.json --await --policy 'REP 1' --nns-name kapusta --nns-zone makaroni.ns --await Enter password > put container rpc error: status: code = 1024 message = frostfs error: chain/client: contract execution finished with state FAULT; exception: at instruction 1274 (THROW): unhandled exception: "domain does not exist or is expired: makaroni.ns" ``` Signed-off-by: Ekaterina Lebedeva <ekaterina.lebedeva@yadro.com>

elebedeva requested review from storage-core-committers 2024-04-16 11:29:29 +00:00

elebedeva requested review from storage-core-developers 2024-04-16 11:29:30 +00:00

elebedeva force-pushed fix/domain-error-msg from 5d255b72d5 to c4a6f82953

2024-04-16 11:43:25 +00:00

Compare

fyrchik requested changes 2024-04-16 11:46:23 +00:00

nns/nns_contract.go Outdated

					
				@ -233,3 +233,3 @@

					}

					if parentExpired(ctx, fragments) {

						panic("parent does not exist or is expired")

						panic("parent of " + name + " does not exist or is expired")

fyrchik commented

2024-04-16 11:45:51 +00:00

parentExpired checks all parents in chain, it would be nice to know exactly which domain does not exist. We can achieve this either by returning index in fragments or replacing parentExpired with checkParentExist inside the function. All parentExpired usages panic anyway.
Similar panic is used in 2 other place, should be handled in a similar fashion.

1. `parentExpired` checks all parents in chain, it would be nice to know exactly which domain does not exist. We can achieve this either by returning index in fragments or replacing `parentExpired` with `checkParentExist` inside the function. All `parentExpired` usages panic anyway. 2. Similar panic is used in 2 other place, should be handled in a similar fashion.

elebedeva commented

2024-04-16 13:42:19 +00:00

Replaced all parentExpired checks with checkParentExist.

Replaced all `parentExpired` checks with `checkParentExist`.

elebedeva force-pushed fix/domain-error-msg from c4a6f82953 to 5381ea1ce4

2024-04-16 13:37:44 +00:00

Compare

elebedeva force-pushed fix/domain-error-msg from 5381ea1ce4 to aef3869ffb

2024-04-16 13:46:58 +00:00

Compare

elebedeva force-pushed fix/domain-error-msg from aef3869ffb to ff4132f165

2024-04-16 13:49:35 +00:00

Compare

aarifullin approved these changes 2024-04-16 14:02:09 +00:00

fyrchik approved these changes 2024-04-16 14:32:56 +00:00

tests/nns_test.go Outdated

					
				@ -350,3 +350,3 @@

					c.Invoke(t, true, "isAvailable", "dom.domain.com")

					c.InvokeFail(t, "parent does not exist or is expired", "isAvailable", "dom.dom.domain.com")

					c.InvokeFail(t, "domain does not exist or is expired: dom.domain.com", "isAvailable", "dom.dom.domain.com")

fyrchik commented

2024-04-16 14:32:54 +00:00

Could you also add this test before this line?

	c1.Invoke(t, true, "register",
		"domain.com", acc.ScriptHash(),
		"myemail@frostfs.info", refresh, retry, expire, ttl)

The error should be the same, but with a different domain ("domain.com").

Could you also add this test before this line? ``` c1.Invoke(t, true, "register", "domain.com", acc.ScriptHash(), "myemail@frostfs.info", refresh, retry, expire, ttl) ``` The error should be the same, but with a different domain ("domain.com").

elebedeva commented

2024-04-16 15:07:54 +00:00

Added.