Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If ACL Handler response error,satck.root_state_accessor_stub.get_object_by_path() will not return #247

Closed
Tracked by #227
lizhihongTest opened this issue Apr 28, 2023 · 8 comments
Assignees
Labels
bug Something isn't working Handler Router Handler System

Comments

@lizhihongTest
Copy link
Collaborator

Describe the bug
If I set ACL handler event return response : cyfs.Err(cyfs.BuckyError.new_dec_error(match_response_error,"request set response_error")), the function will satck.root_state_accessor_stub.get_object_by_path() will not return

zone-simulator_133340_rCURRENT.log

Details

  • hadler
export class DynamicTokenHandler implements cyfs.RouterHandlerAclRoutine{
    private token : string
    constructor(token:string){
        this.token = token;
    }
    async call(param: cyfs.RouterHandlerAclRequest): Promise<cyfs.BuckyResult<cyfs.RouterHandlerAclResult>> {
        console.info(`will handle dynamic acl${param.request.req_path}: query${param.request.req_query_string}`)
        let action = cyfs.AclAction.Reject;
        let querys = param.request.req_query_string.split("&");
        let match_return_error = 0;
        let match_response_error = 0;
        for(let query of querys){
            let [key,value] = query.split("=");
            console.info(`Dynamic Token will check key=${key} value = ${value}`)
            if(key === "token" && value === this.token){
                console.info(`DynamicTokenHandler check token success,will return access accept`)
                action = cyfs.AclAction.Accept;
            }
            if(key === "return_error"){
                match_return_error = Number(value);
                console.info(`DynamicTokenHandler will return error ${match_return_error}`)
                break;
            }
            if(key === "response_error"){
                match_response_error = Number(value);
                console.info(`DynamicTokenHandler will response error ${match_response_error}`)
                break;
            }
        }
        let resp :cyfs.AclHandlerResponse = {
            action
        }
        if(match_response_error != 0){
            return cyfs.Err(cyfs.BuckyError.new_dec_error(match_response_error,"request set return_error"));
        }
        let result : cyfs.RouterHandlerAclResult =  {
            action: cyfs.RouterHandlerAction.Response,
            response : cyfs.Ok(resp),
        }
        if(match_response_error != 0){
            result = {
                action: cyfs.RouterHandlerAction.Response,
                response : cyfs.Err(cyfs.BuckyError.new_dec_error(match_response_error,"request set response_error")),
            }
        }
        return cyfs.Ok(result)
    }  
}
  • client
                let prepare_obj =await new ShareObjectWithTokenAction({
                    local: zone1_ood_app1_http,
                    input: {
                        timeout: 200 * 1000,
                    },
                    expect: { err: 0 },
                }).start({
                    handler_id : "test-the-some-id-handler-789",
                    chain : cyfs.RouterHandlerChain.Acl,
                    root_req_path:"/qa_test_token",
                    index : 0,
                    filter: "*",
                    default_action : cyfs.RouterHandlerAction.Reject,
                    routine : new DynamicTokenHandler(TOKEN),
                    token : TOKEN,
                });
                assert.equal(prepare_obj.err,0,prepare_obj.log);
                let zone1_ood = test_runner.stack_manager.get_cyfs_satck(zone1_ood_app1_http).stack!;
                let zone2_ood = test_runner.stack_manager.get_cyfs_satck(zone2_ood_app1_http).stack!;
                let  get_object_result = await zone2_ood.root_state_accessor_stub(zone1_ood.local_device_id().object_id).get_object_by_path(`${prepare_obj.resp.share_req_path}?token=${TOKEN}&response_error=404`);
                assert.equal( get_object_result.err,true, get_object_result.val.toString());
                if( get_object_result.err){
                    console.info(`get_object_by_path err = ${JSON.stringify(get_object_result.val)}`)
                }

associate issuse #175
@lizhihongTest lizhihongTest added the bug Something isn't working label Apr 28, 2023
@lizhihongTest
Copy link
Collaborator Author

lizhihongTest commented Apr 28, 2023

There are details of this operation rep_path and object

[debug],[2023-04-28 20:14:27.186],<>,put object to non service success: 9tGpLNnLEVgDb664QusWPC1y5P3Hge7CQ5u3ajdFFtDB, cyfs_node.js:81230
[info],[2023-04-28 20:14:27.855],<>,will handle dynamic acl/qa_test_token/share-9tGpLNnLEVgDb664QusWPC1y5P3Hge7CQ5u3ajdFFtDB: querytoken=sk-RnSx7E7HbBEDS9zsiG19ePc02500YS&response_error=404, dynamic_token_handler.ts:33
[info],[2023-04-28 20:14:27.856],<>,Dynamic Token will check key=token value = sk-RnSx7E7HbBEDS9zsiG19ePc02500YS, dynamic_token_handler.ts:40
[info],[2023-04-28 20:14:27.857],<>,DynamicTokenHandler check token success,will return access accept, dynamic_token_handler.ts:42
[info],[2023-04-28 20:14:27.857],<>,Dynamic Token will check key=response_error value = 404, dynamic_token_handler.ts:40
[info],[2023-04-28 20:14:27.858],<>,DynamicTokenHandler will response error 404, dynamic_token_handler.ts:52

cyfs_stack.0.log

@lurenpluto
Copy link
Member

From the processing logs of the acl handler, the error return inside this ts-sdk is not returned to the internal cyfs-stack, probably because there is a problem with the js layer, which causes the request to be blocked and needs to wait for a timeout before it returns

Details

[2023-04-28 20:14:26.293909 +08:00] INFO [ThreadId(4)] [component\cyfs-stack\src\non_api\handler\handler.rs:71] non acl handler resp: chain=acl, category=acl, req=/qa_test_token/share-9tGpLNniyP36axfCsdgDe9ogScdL4mcqXyb31gwsLh4L, action: Response, response: action: Accept
[2023-04-28 20:14:27.139859 +08:00] INFO [ThreadId(10)] [component\cyfs-stack\src\non_api\handler\handler.rs:71] non acl handler resp: chain=acl, category=acl, req=/qa_test_token/share-9tGpLNnVA1mjL3QVJvivQ7XUQt1RJ9zUdCX2xWGKv3fd, action: Response, response: action: Reject

@lizhihongTest
Copy link
Collaborator Author

@weiqiushi If DynamicTokenHandler.call() return cyfs.Err() has the some issuse.

@weiqiushi
Copy link
Member

The processing logic here is:

When the call() of Routine returns cyfs.Err(), it means that an error occurred in the processing function itself, and this error will not be returned to the calling end.
Only when call() returns cyfs.Ok(resp), the processing result, as indicated by resp, will be returned to the calling end.

The above processing logic is common to all handlers

back to this issue, you MUST return Ok({action: cyfs.AclAction.Reject}) when you want caller recv a acl reject error.

@weiqiushi weiqiushi added the invalid This doesn't seem right label May 4, 2023
@lurenpluto
Copy link
Member

The processing logic here is:

When the call() of Routine returns cyfs.Err(), it means that an error occurred in the processing function itself, and this error will not be returned to the calling end. Only when call() returns cyfs.Ok(resp), the processing result, as indicated by resp, will be returned to the calling end.

The above processing logic is common to all handlers

back to this issue, you MUST return Ok({action: cyfs.AclAction.Reject}) when you want caller recv a acl reject error.

The current user layer registered Handler, in the case of return Err (), is equivalent to the SDK internal processing error, this case does not do a timely response to the caller, so need to wait for a timeout before returning, this is not very friendly, need to improve

Consider in the ws-based event system, the unified introduction of such errors error packet to deal with, that the event response side of the error, so that the upper layer can receive timely feedback and continue the subsequent process

@lurenpluto lurenpluto added Handler Router Handler System and removed invalid This doesn't seem right labels May 4, 2023
lurenpluto added a commit that referenced this issue May 5, 2023
…state_accessor_stubget_object_by_path-will-not-return' into main
@lurenpluto
Copy link
Member

This logical improvement: In the case that the handler callback returns Err(e), the processing is considered to have failed, which is equivalent to the failure of the entire processing logic of the callback, using the default action of the handler's registration as the processing result

To handle this case, a WS_CMD_PROCESS_ERROR cmd is added to the handler's ws protocol to return an error

// An error occurred while processing the request
pub const WS_CMD_PROCESS_ERROR: u16 = u16::MAX;

@lizhihongTest
Copy link
Collaborator Author

@lurenpluto cyfs ts-sdk reported an error when codec.encode_string

[info],[2023-05-05 13:23:46.764],<>,DynamicTokenHandler check token success,will return access accept, dynamic_token_handler.ts:42
[info],[2023-05-05 13:23:46.765],<>,Dynamic Token will check key=return_error value = 502, dynamic_token_handler.ts:40
[info],[2023-05-05 13:23:46.766],<>,DynamicTokenHandler will return error 502, dynamic_token_handler.ts:58
[error],[2023-05-05 13:23:46.768],<>,ws process packet error: sid=838, err= TypeError: codec.encode_string is not a function, cyfs_node.js:88506
[error],[2023-05-05 13:28:46.446],<>,fetch error: undefined, fetch failed, cyfs_node.js:76984

@lizhihongTest
Copy link
Collaborator Author

This issuse has been fixed,it will return error success.

get_object_by_path err = {"m_code":{"m_code":30,"m_value":30},"m_msg":"non acl rejected default or by handler! chain=acl, category=acl, req=/qa_test_token/share-9tGpLNnWdacZ1tSsVh4y34c8vZFqibsbge2p5bUK3bdx"}

streetycat pushed a commit to streetycat/CYFS that referenced this issue May 12, 2023
…ckroot_state_accessor_stubget_object_by_path-will-not-return' into main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Handler Router Handler System
Projects
Status: Done
Development

No branches or pull requests

3 participants