sdk_review

## Review Points

### **Binding to `window`**

The SDK binds to the `window` object, which both restricts and presumes the usage is to be in the context of a browser. Which does the following:

- Makes the implementer rightly assume this code is to only be used in the browser context which should not be done in its current state, as will be outlined 
- Does not allow use without polyfilling window in a Node.JS environment (Common for API <-> API implementation)
- Due to these presumptions this creates security concerns and attack vectors that will be outlined further below

### **Credential Concerns**

Requiring credentials like `username` and `password` directly in the configuration makes the SDK vulnerable on various fronts:

- Being used in browser means both username and password are readily accessible to the user and anyone on the machine and makes them viable targets for XSS attacks.

### **No Out of Box Support for Secure Environments** 

Since it relies on `window`, this SDK won't function in server-side environments such as Node.js without polyfills. This severely limits its flexibility and forces developers to use insecure patterns or add additional complexity just to use the SDK in its current state.

### **Mixed Asynchronous Code**

`async/await` functions are wrapped in unnecessary Promises which adds redundancy and makes the code more complex and difficult to read by using a mix of Promise handling logic.

### **Redundant Error Handling**
The error handling mechanism is verbose and inconsistent. In several places, errors are rethrown as generic JavaScript errors, losing context about the underlying issue. 

This will make debugging difficult for developers using the SDK due to specific API-related errors possibly not propagating correctly.

An example of how errors from call would potentially need handled depending on if they were thrown with a payload or Error object

 -- This point is even alluded to in the code as multiple comments in the error handling

```
    .then(function(content) {
        // Handle success
    })
    .catch(function(error) {
        // Check if its a native Error first...
        if (error instanceof Error) {
            console.log("Network error:", error.message); // Native Error object
        } else {
        // Handle custom json err..
            console.log("API error:", error.message, error.status);
        }
```

### **Code Duplication and Poor Reusability** 

Many functions, such as `create`, `createAsset`, and `createModel`, follow the same structure, passing identical parameters to `call()`. 

These should be consolidated into a more reusable structure, reducing boilerplate and improving readability.

### **No Token Refresh Logic**

The token refresh logic is incomplete, and expired tokens are not handled properly.

Currently when the token expires and the SDK gets a `401` error, it tries to re-authenticate by calling `auth()`. This may create unnecessary multiple calls, leading to race conditions or performance degradation.*

*This is also noted as a comment in the existing codebase and has not been addressed

### **No Modular Packaging**

It was passed to us as a flat js file and not distributed as a modern ES6 or CommonJS (CJS) module, which is standard for modern day JavaScript libraries. 

SDKs should support module imports via `import` or `require`, and not expect to be bound unnecessarily to globals making this approach outdated and cumbersome for developers using modern build tooling such as webpack, vite, etc.

It removes the ability to update the package seamlessly through semantic version notation and package management tools. This will instead force developers to replace flat files of handed off libraries which opens up many avenues for user error.

### **No TypeScript Typing** 
There is no type safety in this SDK. The absence of TypeScript typings means it is more prone to runtime errors, and harder to use in large, strongly-typed codebases like we are writing. 

In modern development environments, a lack of type definitions makes this SDK difficult to implement or maintain.

### **Versioning Concerns**

It is assumed this SDK uses a level of semantic versioning as it is versioned at 2.0.0, the current state of the code suggests a lack of attention to security, modularity, and code cleanliness, which raises concerns about the number of builds this SDK has gone through without addressing the mentioned issues.

## Contrast to GCP SDKs

In comparison to more other SDKs like those from GCP:

**Security Practices**

Use industry-standard OAuth authentication flows and strongly discourage passing credentials directly in the code. This SDK, by contrast, uses a risky approach of requiring `username` and `password` directly in the configuration.
  
**Modular Architecture**

Packaged as modern ES6 or CommonJS modules and can be easily integrated into both browser and Node.js environments as they are availabel via NPM. 

This SDK does not adhere to such modular standards, locking it into an insecure and outdated model.

**Efficient Async Handling**

Proper use of `async/await`, without redundant Promise wrapping, ensuring cleaner and more efficient asynchronous code execution. 

This SDK's mix of async/await and Promises adds unnecessary complexity and potential bugs.

**Token Management**

Include token management, handling token expiration  transparently for the implementing developers. 

In contrast, this SDK's incomplete token handling introduces the risk of unexpected failures and poor user experience during implementation

## SDK Summary

- Suggests *highly insecure* methods due to its binding to the `window` object, requiring credentials directly, and lacking proper token management.
- It is outdated in terms of packaging and structure, failing to support modern JavaScript environments and modules.
- The async code is complex, making the SDK difficult to maintain and increases the likelihood of integration and further development issues.
- The error handling is inconsistent and lacks clarity, which can lead to a poor developer experience when debugging
- It is not typed, lacking TypeScript support, making it error-prone and difficult to maintain and implement.