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

OpEnvRequestor printed log of OpEnv type is incorrect #14

Open
lizhihongTest opened this issue Apr 3, 2023 · 1 comment
Open

OpEnvRequestor printed log of OpEnv type is incorrect #14

lizhihongTest opened this issue Apr 3, 2023 · 1 comment
Assignees
Labels
bug Something isn't working SDK-Tool This is SDK Tool

Comments

@lizhihongTest
Copy link

Describe the bug
IsolatePath/OpEnvStubSingleOpEnvStub/PathOpEnvStub all use GlobalStateRequestor to send requests, and the logs printed by GlobalStateRequestor should be controlled by parameters instead of randomly printing OpEnv types

[[cyfs-ts-sdk] https://github.com/buckyos/cyfs-ts-sdk/src/sdk/cyfs-lib/root_state/requestor.ts](https://github.com/buckyos/cyfs-ts-sdk/blob/beta/src/sdk/cyfs-lib/root_state/requestor.ts)

To Reproduce
For example : IsolatePath/PathOpEnvStub can call this function,why the log just print path_op_env ,This is a bad log, which will cause the user to view the log and cannot distinguish the OpEnv type

public async load(req: OpEnvLoadOutputRequest): Promise<BuckyResult<void>> {
        if (this.op_env_type_ !== ObjectMapOpEnvType.Single && this.op_env_type_ !== ObjectMapOpEnvType.IsolatePath) {
            const err_msg = `load method only valid for single_op_env and isolate_path_op_env! sid = ${this.sid_}`;
            console.log(err_msg);
            return Ok(undefined);
        }

        console.info(
            `will load for path_op_env: sid=${this.sid_}, target=${req.target}`
        );

        const http_req = this.encode_load_request(req);
        const r = await this.requestor_.request(http_req);
        if (r.err) {
            console.error(
                `load for path_op_env request failed: sid=${this.sid_}, target=${req.target}, ret=${r}`
            );
            return r;
        }

        const resp = r.unwrap();
        if (http_status_code_ok(resp.status)) {
            console.info(
                `load for path_op_env success: sid=${this.sid_}, target=${req.target}`
            );

            return Ok(undefined);
        } else {
            const e = await RequestorHelper.error_from_resp(resp);
            console.error(
                `load for path_op_env failed: sid=${this.sid_}, target=${req.target}, err=${e}`
            );

            return Err(e);
        }
    }
  • The other functions of the class OpEnvRequestor have the same problem

Expected behavior
OpEnvRequestor can identify the specific OpEnv type through op_env_type_, it is recommended to optimize the log

System information
[[cyfs-ts-sdk] https://github.com/buckyos/cyfs-ts-sdk/src/sdk/cyfs-lib/root_state/requestor.ts](https://github.com/buckyos/cyfs-ts-sdk/blob/beta/src/sdk/cyfs-lib/root_state/requestor.ts)

@lizhihongTest lizhihongTest added the bug Something isn't working label Apr 3, 2023
@weiqiushi weiqiushi transferred this issue from buckyos/CYFS Apr 3, 2023
@lurenpluto lurenpluto moved this to 🐞Discovered Bugs in CYFS-Stack & Services Apr 3, 2023
@lurenpluto lurenpluto added the SDK-Tool This is SDK Tool label Apr 3, 2023
@weiqiushi
Copy link
Member

This optimization is more difficult to do:

Currently, ObjectMapOpEnvType in this.op_env_type_ used to communicate CYFS Stack with the CYFS protocol, its value is not designed for reading. If you use ObjectMapOpEnvType in the log, readability will be significantly reduced.

For example: will load for path_op_env will become will load for path, or will load for single.

Adding a prefix to the log to optimize readability will only turn it into will load for op_env type path, or will load for op_env type isolate-path.

I'm still hesitant to change this

@weiqiushi weiqiushi moved this from 🐞Discovered Bugs to 🅿️Blocked/Parked in CYFS-Stack & Services Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working SDK-Tool This is SDK Tool
Projects
Status: 🅿️Blocked/Parked
Development

No branches or pull requests

3 participants