nspcc-dev/neofs-node

Fork 0

mirror of https://github.com/nspcc-dev/neofs-node.git synced 2026-03-01 04:29:10 +00:00

Reconsider status responses #1188

New issue

Open

opened 2025-12-28 17:22:07 +00:00 by sami · 7 comments

sami commented

2025-12-28 17:22:07 +00:00

Owner

Originally created by @carpawell on GitHub (Feb 21, 2024).

Some status errors in NeoFS are known and self-describing. But there are also "general errors" with 1024 status codes and some "good" human-readable messages.

Expected Behavior

If I have a general error, I expect NOT a general message that can help me understand what is happening. E.g. session token does not relate to the container a request is trying to change:

status: code = 1024 message = could not execute SetEACL request: session token validation: wrong container:

Current Behavior

Only the last error in the errors chain is attached:

status: code = 1024 message = wrong container:

Possible Solution

errors.Join for meaningful errros inside a package? (like session tokens errors can be wrapped with a common session token error with detailed errors so the Unwrap does not unwrap more than it is needed as the package thinks)
Make all the low-level errors well described and have more context than they have now? (can be a huge work)
Attach the full error chain if it is a 1024 error? (chain could be too long and too golang specific)
Attach 1-2 more errors from the lowest one? (a strange rule can fix smth but not all the cases)

Steps to Reproduce (for bugs)

Any 1024 "general" error case (like incorrect session token; even though session token error may become another status case, there always be some "general" errors that should be described correctly).

Context

nspcc-dev/neofs-node@996b1eae7b/pkg/services/util/sign.go (L225-L229)

Regression

No.

Your Environment

0.40.0

Originally created by @carpawell on GitHub (Feb 21, 2024). Some status errors in NeoFS are known and self-describing. But there are also "general errors" with 1024 status codes and some "good" human-readable messages. ## Expected Behavior If I have a general error, I expect NOT a general message that can help me understand what is happening. E.g. session token does not relate to the container a request is trying to change: ``` status: code = 1024 message = could not execute SetEACL request: session token validation: wrong container: ``` ## Current Behavior Only the last error in the errors chain is attached: ``` status: code = 1024 message = wrong container: ``` ## Possible Solution 1. [errors.Join](https://go.dev/pkg/errors/#Join) for meaningful errros inside a package? (like session tokens errors can be wrapped with a common session token error with detailed errors so the `Unwrap` does not unwrap more than it is needed as the package thinks) 2. Make all the low-level errors well described and have more context than they have now? (can be a huge work) 3. Attach the full error chain if it is a 1024 error? (chain could be too long and too golang specific) 4. Attach 1-2 more errors from the lowest one? (a strange rule can fix smth but not all the cases) ## Steps to Reproduce (for bugs) Any 1024 "general" error case (like incorrect session token; even though session token error may become another status case, there always be some "general" errors that should be described correctly). ## Context https://github.com/nspcc-dev/neofs-node/blob/996b1eae7bd1f1ad5693758b38c9c025523842ac/pkg/services/util/sign.go#L225-L229 ## Regression No. ## Your Environment 0.40.0

sami added the

discussion

labels

2025-12-28 17:22:07 +00:00

sami commented

2025-12-28 17:22:07 +00:00

Author

Owner

@roman-khimov commented on GitHub (Feb 21, 2024):

Server should return as much data as it can (except when it somehow affects security), simple as that. Joining/chaining errors is fine for that and the protocol allows for it, message can be anything. Then this message should be a part of client error.

@roman-khimov commented on GitHub (Feb 21, 2024): Server should return as much data as it can (except when it somehow affects security), simple as that. Joining/chaining errors is fine for that and the protocol allows for it, `message` can be anything. Then this message should be a part of client error.

sami commented

2025-12-28 17:22:07 +00:00

Author

Owner

@carpawell commented on GitHub (Feb 21, 2024):

@roman-khimov, so you are for the full error message?

@carpawell commented on GitHub (Feb 21, 2024): @roman-khimov, so you are for the full error message?

sami commented

2025-12-28 17:22:07 +00:00

Author

Owner

@roman-khimov commented on GitHub (Feb 21, 2024):

Absolutely. It's not that much different from https://github.com/neo-project/proposals/pull/156

@roman-khimov commented on GitHub (Feb 21, 2024): Absolutely. It's not that much different from https://github.com/neo-project/proposals/pull/156

sami commented

2025-12-28 17:22:07 +00:00

Author

Owner

@carpawell commented on GitHub (Feb 21, 2024):

Well, I agree too, I think. I just try to understand why it was done like that. I think the idea was that "a user do not want to see Invalid table argument to Dexie.transaction(). Only Table or String are allowed [Caught in: [MWPTransactor] Error performing loadExpired transaction]" (nicely copied from the browser's console) errors in the response. Maybe.

@carpawell commented on GitHub (Feb 21, 2024): Well, I agree too, I think. I just try to understand why it was done like that. I think the idea was that "a user do not want to see `Invalid table argument to Dexie.transaction(). Only Table or String are allowed [Caught in: [MWPTransactor] Error performing loadExpired transaction]`" (nicely copied from the browser's console) errors in the response. Maybe.

sami commented

2025-12-28 17:22:07 +00:00

Author

Owner

@roman-khimov commented on GitHub (May 7, 2024):

1024 in ContainerDelete delete: status: code = 1024 message = session was not issued by the container owner, issuer: NehJedoS24C7LkeJCzSFpApivRmHKJe1Q6 is rather strange. 2048 is almost perfect except it's defined for object operations.

@roman-khimov commented on GitHub (May 7, 2024): 1024 in `ContainerDelete delete: status: code = 1024 message = session was not issued by the container owner, issuer: NehJedoS24C7LkeJCzSFpApivRmHKJe1Q6` is rather strange. 2048 is almost perfect except it's defined for object operations.

sami commented

2025-12-28 17:22:07 +00:00

Author

Owner

@cthulhu-rider commented on GitHub (Dec 26, 2024):

big 👍 to this proposal

pretty simple approach comes to my mind:

server always returns full error context in text message, i.e. drop mentioned pre-unwrapping
CLI checks the code first, and prints clear output according to it. If there are some details, CLI adds them just like we add context to Go errors. For example, for 2048 w/o details, CLI just types:

2048 Fobidden

or, if status has detail:

2048 Fobidden
access to operation OBJECT_PUT is denied by basic ACL check

and no other info. See next
3. with -v, command also prints as now:

rpc error: finish object stream: status: code = 2048 message = access to object operation denied: access to operation OBJECT_PUT is denied by basic ACL check

i.e. code + all details + full text message
4. for 1024, it makes sense to always print full message:

Internal server error: <error1: error2: error3...>

and with -v

rpc error: finish object stream: status: code = 2048 message = <error1: error2: error3...>

finally, user faces no transport/proto details and strange colon-separated texts in widely-known cases like Not found or Fobidden unless -v

will also help with https://github.com/nspcc-dev/neofs-testcases/issues/919

i faced in tests:

AssertionError: Regex pattern did not match.
 Regex: 'code = 1024.*message = found link object'
 Input: 'Command: ./neofs-cli --config /home/runner/work/neofs-node/neofs-node/neofs-testcases/wallet_config.yml object delete --rpc-endpoint \'localhost:38811\' --wallet \'/home/runner/work/neofs-node/neofs-node/neofs-testcases/test-run-2024-12-26-09-42-07-773832/wallet-5023f627-acfc-481e-afd3-53d81d5c07d7.json\' --cid \'FHj84xwDksPCvnF5A7PZMaD6QkLiZQpjTxLaR6CxgzSg\' --oid \'9gbENs5mEXtiJD8jKUfoB4QxpQgBDbAeYVmnmc79ctFn\'\nError:\nreturn code: 1\noutput: rpc error: deleting "+addr.Object().String()+" object: remove object via client: status: code = 1024 message = (*putsvc.Streamer) could not close object target: finish object slicing: write formed object: finish object stream: (*putsvc.distributedTarget) could not validate payload content: tombstone verification: verifying 9gbENs5mEXtiJD8jKUfoB4QxpQgBDbAeYVmnmc79ctFn member: verify V2 split: found link object 9gbENs5mEXtiJD8jKUfoB4QxpQgBDbAeYVmnmc79ctFn\n

this is what becomes when unwrapping is dropped. Realized 2 facts:

as regular user, such text would horrify me
as dev, i'd need it full

combining proposed default simplification and -v will help to everybody

@cthulhu-rider commented on GitHub (Dec 26, 2024): big :+1: to this proposal pretty simple approach comes to my mind: 1. server always returns full error context in text message, i.e. drop mentioned pre-unwrapping 2. CLI checks the code first, and prints clear output according to it. If there are some details, CLI adds them just like we add context to Go errors. For example, for [`2048`](https://github.com/nspcc-dev/neofs-api/blob/e360d265f85182fc19aa890b5b26b65430ff9018/status/types.proto#L108-L111) w/o details, CLI just types: ``` 2048 Fobidden ``` or, if status has detail: ``` 2048 Fobidden access to operation OBJECT_PUT is denied by basic ACL check ``` and no other info. See next 3. with `-v`, command also prints as now: ``` rpc error: finish object stream: status: code = 2048 message = access to object operation denied: access to operation OBJECT_PUT is denied by basic ACL check ``` i.e. code + all details + full text message 4. for `1024`, it makes sense to _always_ print full message: ``` Internal server error: <error1: error2: error3...> ``` and with `-v` ``` rpc error: finish object stream: status: code = 2048 message = <error1: error2: error3...> ``` finally, user faces no transport/proto details and strange colon-separated texts in widely-known cases like `Not found` or `Fobidden` unless `-v` will also help with https://github.com/nspcc-dev/neofs-testcases/issues/919 --- i faced in tests: ``` AssertionError: Regex pattern did not match. Regex: 'code = 1024.*message = found link object' Input: 'Command: ./neofs-cli --config /home/runner/work/neofs-node/neofs-node/neofs-testcases/wallet_config.yml object delete --rpc-endpoint \'localhost:38811\' --wallet \'/home/runner/work/neofs-node/neofs-node/neofs-testcases/test-run-2024-12-26-09-42-07-773832/wallet-5023f627-acfc-481e-afd3-53d81d5c07d7.json\' --cid \'FHj84xwDksPCvnF5A7PZMaD6QkLiZQpjTxLaR6CxgzSg\' --oid \'9gbENs5mEXtiJD8jKUfoB4QxpQgBDbAeYVmnmc79ctFn\'\nError:\nreturn code: 1\noutput: rpc error: deleting "+addr.Object().String()+" object: remove object via client: status: code = 1024 message = (*putsvc.Streamer) could not close object target: finish object slicing: write formed object: finish object stream: (*putsvc.distributedTarget) could not validate payload content: tombstone verification: verifying 9gbENs5mEXtiJD8jKUfoB4QxpQgBDbAeYVmnmc79ctFn member: verify V2 split: found link object 9gbENs5mEXtiJD8jKUfoB4QxpQgBDbAeYVmnmc79ctFn\n ``` this is what becomes when unwrapping is dropped. Realized 2 facts: 1. as regular user, such text would horrify me 2. as dev, i'd need it full combining proposed default simplification and `-v` will help to everybody

sami commented

2025-12-28 17:22:07 +00:00

Author

Owner

@cthulhu-rider commented on GitHub (Jul 15, 2025):

when PUT server accepts LINK object and verifies its content, it may not found some split member. Here what we get:

$ neofs-cli object put -r s01.neofs.devenv:8080 -w ../devenv/wallets/wallet.json --cid BKVC2YsNUoHCRYLM75RX3dCPPA5Fk6xdArgV2VtfjvSH --file Makefile --no-progress
Enter password > 
rpc error: finish object stream: status: code = 2049 message = object not found

getting 404 to PUT is confusing. I checked original server error

could not object put stream: (*putsvc.Streamer) could not close object target: finish object slicing: write linking object: finish object stream: (*putsvc.distributedTarget) could not validate payload content: link object's split chain verification: reading BKVC2YsNUoHCRYLM75RX3dCPPA5Fk6xdArgV2VtfjvSH/A4oPoFZWRwbrRj7U799VXvzu8eVdKvvTZ1Xbsaw9W4pX header: status: code = 2049 message = object not found

@cthulhu-rider commented on GitHub (Jul 15, 2025): when PUT server accepts LINK object and verifies its content, it may not found some split member. Here what we get: ``` $ neofs-cli object put -r s01.neofs.devenv:8080 -w ../devenv/wallets/wallet.json --cid BKVC2YsNUoHCRYLM75RX3dCPPA5Fk6xdArgV2VtfjvSH --file Makefile --no-progress Enter password > rpc error: finish object stream: status: code = 2049 message = object not found ``` getting 404 to PUT is confusing. I checked original server error ``` could not object put stream: (*putsvc.Streamer) could not close object target: finish object slicing: write linking object: finish object stream: (*putsvc.distributedTarget) could not validate payload content: link object's split chain verification: reading BKVC2YsNUoHCRYLM75RX3dCPPA5Fk6xdArgV2VtfjvSH/A4oPoFZWRwbrRj7U799VXvzu8eVdKvvTZ1Xbsaw9W4pX header: status: code = 2049 message = object not found ```