simojenki / bonob

sonos SMAPI implementation allowing integrating different music sources with sonos.
GNU General Public License v3.0
222 stars 16 forks source link

Object is possibly 'null' when trying to build #208

Open gravelld opened 3 months ago

gravelld commented 3 months ago

I cloned the repo (master) and ran:

$ node -v
v20.15.1

So then I did:

$ npm run build

> bonob@0.0.1 build
> tsc

src/encryption.ts:31:33 - error TS2531: Object is possibly 'null'.

31     decrypt: (value: string) => jws.decode(value).payload
                                   ~~~~~~~~~~~~~~~~~

Found 1 error in src/encryption.ts:31

I guess some sort of version issue somewhere.

Note the Docker container builds and runs fine, even though it's the same version of node internally.

simojenki commented 3 months ago

Are you running inside the devcontainer? I just rebuilt the container and everything seems to work ok...

gravelld commented 3 months ago

No, on the host. The container appears to copy and rebuild at build time rather than have the source mounted and rebuilding as files are updated, which makes for a really long round trip experience. Is there an alternative image?

simojenki commented 3 months ago

I'm guessing you mean when you want to spin it up to test it against a real sonos device? My workflow is to start it up in a vscode devcontainer, and then i run either;

To just start it up, ie. you have already registered a dev instance of bonob with your sonos devices on an IP that sonos can find. npm run dev

To start it up and register with sonos I run. Note that this will register it with z_bonobDev (so it appears at the bottom of the list in the sonos UI) npm run devr

gravelld commented 3 months ago

I don't need to register it with Sonos. I was just wanting to test the SMAPI handling to add support for returning a 200 and the required XML for reportAccountAction (#205 ).

I've not used VS Code Dev Containers before but I can look at that. As it runs ok in a container maybe this is the key, whereas something in my Node setup on my host is screwed. My only concern was the round trip - I did start trying to rebuild the container after each edit of .ts files but it takes a few minutes each time which isn't really tolerable for ongoing development.

simojenki commented 3 months ago

vscode dev containers should give you the feedback loop you are looking for, that is how i develop it.

gravelld commented 2 months ago

Thanks. I followed the instructions here: https://code.visualstudio.com/docs/devcontainers/containers and the container started first time. I've got another problem but I'll open a separate ticket for that.

gravelld commented 2 months ago

Sorry to re-open, but I realised after posting #209 that running these npm targets might be required from within the dev container.

I think I expected the bonob server to run as the CMD or ENTRYPOINT in the container, but my current theory is that it's just dev container stuff that does this plumbing.

So I tried: npm run dev

But I got the same original issue within the dev container:

root@2e3308821332:/workspaces/bonob# npm run dev

> bonob@0.0.1 dev
> BNB_SUBSONIC_CUSTOM_CLIENTS1=audio/flac,audio/mpeg,audio/mp4\>audio/flac BNB_LOG_LEVEL=debug BNB_DEBUG=true BNB_SCROBBLE_TRACKS=false BNB_REPORT_NOW_PLAYING=false BNB_SONOS_SEED_HOST=$BNB_DEV_SONOS_DEVICE_IP BNB_SONOS_SERVICE_NAME=z_bonobDev BNB_URL="http://${BNB_DEV_HOST_IP}:4534" BNB_SUBSONIC_URL="${BNB_DEV_SUBSONIC_URL}" nodemon -V ./src/app.ts

[nodemon] 3.1.4
[nodemon] to restart at any time, enter `rs`
[nodemon] or send SIGHUP to 4375 to restart
[nodemon] watching path(s): *.*
[nodemon] watching extensions: ts,json
[nodemon] starting `ts-node ./src/app.ts`
[nodemon] spawning
[nodemon] child pid: 4388
[nodemon] watching 201 files
/workspaces/bonob/node_modules/ts-node/src/index.ts:859
    return new TSError(diagnosticText, diagnosticCodes, diagnostics);
           ^
TSError: ⨯ Unable to compile TypeScript:
src/encryption.ts:31:33 - error TS2531: Object is possibly 'null'.

31     decrypt: (value: string) => jws.decode(value).payload
                                   ~~~~~~~~~~~~~~~~~

    at createTSError (/workspaces/bonob/node_modules/ts-node/src/index.ts:859:12)
    at reportTSError (/workspaces/bonob/node_modules/ts-node/src/index.ts:863:19)
    at getOutput (/workspaces/bonob/node_modules/ts-node/src/index.ts:1077:36)
    at Object.compile (/workspaces/bonob/node_modules/ts-node/src/index.ts:1433:41)
    at Module.m._compile (/workspaces/bonob/node_modules/ts-node/src/index.ts:1617:30)
    at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
    at Object.require.extensions.<computed> [as .ts] (/workspaces/bonob/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Function.Module._load (node:internal/modules/cjs/loader:1024:12)
    at Module.require (node:internal/modules/cjs/loader:1233:19) {
  diagnosticCodes: [ 2531 ]
}
[nodemon] app crashed - waiting for file changes before starting...

I've tried the above both from the VS Code Terminal into the container, and running docker exec -it in a separate terminal (the above output is from the latter, hence root@).

I don't really fully understand the full development process - I just thought it was run a container, then make changes to the code and it will recompile. But it seems like, as above, I have to manually run bonob from within the container in addition, is that correct? How are the env vars supposed to be plumbed through in this case? Do you modify that in devcontainer.json or is there some way of parameterising the image run?

gravelld commented 2 months ago

I just added a null check, this fixes it...

-    decrypt: (value: string) => jws.decode(value).payload
+    decrypt: (value: string) => jws.decode(value)!.payload

But I'm puzzled why you aren't getting the same issue @simojenki - feels like we have a different configuration somewhere along the lines.

gravelld commented 1 month ago

(You should close this is you want btw, don't feel you have to keep it open - I just thought you might want to look into the developer experience).

simojenki commented 1 month ago

I'm happy to leave this open for now as my intention is to come back and address these issues.