balena-io / balena-sdk

The SDK to make balena powered JavaScript applications.
https://www.balena.io/
Apache License 2.0
146 stars 46 forks source link

The SDK doesn't throw errors when needed #1298

Closed bartversluijs closed 1 year ago

bartversluijs commented 1 year ago

Expected Behavior

I expect when a request fails or doesn't return the value it should, it throws an error, which can be handled with a catch.

Actual Behavior

The actual behavior is that the SDK doesn't error and the response is a string of the error. I've seen it for quite some requests. For example, when executing the auth.whoami() function with invalid credentials, I get an undefined response instead of a user. This causes the auth.isLoggedIn() function to always state true, which isn't the case.

Next example is creating an application. I created an application using models.application.create(), but the name contained an invalid character. So I expected an error. But what I got was a string in the response stating the error. This caused my Node.js application to just accepting it and moving in instead of throwing an error.

Steps to Reproduce the Problem

Note Tested this with openBalena.

  1. Install the SDK
  2. Logging in with credentials or a token
  3. Executing the models.application.create() function with an invalid name.

Specifications

References

thgreasi commented 1 year ago

Hi, can you provide an example with the arguments that you provide to the .create() call? I would be interested at seeing the invalid name that causes this issue.

bartversluijs commented 1 year ago

Hi,

I used a colon (:) in the name, which caused the fail. So something like fleet:test.

thgreasi commented 1 year ago

Hi @bartversluijs, It seems that this fell through the cracks of my inbox...

I think the issue that you are facing is that you are not waiting for the returned promise to resolve.

This causes the auth.isLoggedIn() function to always state true, which isn't the case.

auth.isLoggedIn() returns a promise, which if used directly in an if statement could be confused with a true boolean, but if you console.log(sdk.auth.isLoggedIn()) your console should print Promise {<pending>}. You can use .then() or await inside async functions. Let me also point you to our test cases that make sure that the returned promise resolves to false when there is no logged in user: https://github.com/balena-io/balena-sdk/blob/dde3a133a6a79c36c0c3d9c781ffaa1614ada125/tests/integration/auth.spec.ts#L19-L23

Similarly when calling application.create() with invalid parameters, the function will not throw synchronously (especially since the name validation is done by our backend), but it will instead return a promise that will be rejected. You can handle that case by either using .catch((err) => { console.log(err) }) or using try catch with await like:

try {
    const fleet = await sdk.models.application.create({
        deviceType: 'raspberrypi3',
        name: 'fleet:test',
        organization: 'thgreasinomembersorg'
    });
    console.log(fleet);
} catch (err) {
    console.error(err);
}

which should log something like:

BalenaRequestError: Request error: App name may only contain [a-zA-Z0-9_-].

Given that we do have test cases for these I will close the issue for now, but feel free to re-open or ask for further help here or in a new issue.

Kind regards, Thodoris