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

Add strict mode to runtime-rest-client and runtime-lifecycle #166

Closed
hrajchert opened this issue Jan 11, 2024 · 2 comments
Closed

Add strict mode to runtime-rest-client and runtime-lifecycle #166

hrajchert opened this issue Jan 11, 2024 · 2 comments
Assignees

Comments

@hrajchert
Copy link
Collaborator

As mentioned in this discussion response:

We use the runtime types to validate external values that comes from the REST client, as a sanity check. But we don't use them from the user facing API as it would add some unwanted overhead. This means that if you are using the SDK with JS and you call runtimeLifecycle.contracts.createContract with a wrong contract or role token configuration, the SDK will manipulate or pass along the wrong values to the backend, yielding an unexpected error. I was thinking of adding a strict property when you create a runtime lifecycle to do this validation at the surface, maybe this can inprove the DX for JS users.

The idea is to modify mkRestClient and mkRuntimeLifecycle to accept a strict property that has false as the default value.

When the property is true, all calls to those API will try the guard on the input and throw a specific error (InvalidInputs/BadCall/error-to-be-named).

We could take this opportunity to also make a breaking change in the restClient so all functions accept a single object parameter. Right now we have some functions that accept a single Request parameter and some functions that accept multiple ones, this would simplify the API.

@bjornkihlberg
Copy link
Contributor

We shouldn't have strict mode be false by default, it should be true by default. Otherwise it's one more thing a new developer using either JavaScript or poorly typed TypeScript will have to know about to get their code working well.

@bjornkihlberg bjornkihlberg self-assigned this Jan 16, 2024
@bjornkihlberg
Copy link
Contributor

bjornkihlberg commented Jan 24, 2024

Notes

Progress

  • Draft PR: PR-180
    • Ensure the rest client flow works
    • Ensure all new keyword argument objects are touched by documentation
    • Ensure test suite passes
    • Make PR
  • Locate mkRestClient
  • Locate mkRuntimeLifecycle
    • There are four definitions, which one?
      • A: ./packages/runtime/lifecycle/src/generic/runtime.ts
      • B: ./packages/runtime/lifecycle/src/browser/index.ts
        • Invokes A
      • C: ./packages/runtime/lifecycle/src/index.ts
        • Invokes A
      • D: ./packages/runtime/lifecycle/src/nodejs/index.ts
        • Invokes A
      • A is the answer
        • Actually, runtime checks on B, C and D for their respective parameters is required, not A.
  • Unify all methods for RestClient to use keyword arguments rather than positional arguments
  • Put strictness parameter on mkRestClient
    • Locate interesting points of runtime checking
      • Start interactive session and start passing junk
      • Multimethod approach?
        function mkRestClient(baseURL: string): RestClient
        function mkRestClient(baseURL: string, strict: boolean): RestClient
        function mkRestClient(baseURL: unknown, strict: unknown = true): RestClient
      • Use healthcheck to report whether the baseURL is sound and that runtime could be accessed
        • This would require mkRestClient to be async which would be a breaking change. A better error message can still be provided if the user has provided incorrect runtime URL.
        • Use CompatibleRuntimeVersionGuard to check the runtime version
      • getContracts
      • buildCreateContractTx
      • createContractSources
        • use keyword arguments rather than positional arguments
      • getContractSourceById
      • getContractSourceAdjacency
      • getContractSourceClosure
      • getNextStepsForContract
      • getContractById
        • use keyword arguments rather than positional arguments
      • submitContract
        • use keyword arguments rather than positional arguments
      • getTransactionsForContract
        • use keyword arguments rather than positional arguments
      • applyInputsToContract
      • submitContractTransaction
        • use keyword arguments rather than positional arguments
      • getContractTransactionById
        • use keyword arguments rather than positional arguments
      • withdrawPayouts
      • getWithdrawals
      • getWithdrawalById
        • use keyword arguments rather than positional arguments
      • submitWithdrawal
        • use keyword arguments rather than positional arguments
      • getPayouts
      • getPayoutById
  • Put strictness parameter on mkRuntimeLifecycle (B)
  • Put strictness parameter on mkRuntimeLifecycle (C)
  • Put strictness parameter on mkRuntimeLifecycle (D)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants