coapjs / node-coap

CoAP - Node.js style
MIT License
531 stars 154 forks source link

dotenv conflict with latest version #328

Closed invaderb closed 2 years ago

invaderb commented 2 years ago

Hey guys,

I noticed on a new project I'm setting up that dotenv is having an issue with the latest version on node-coap, with dotenv preloaded requests no longer come in. "dev": "node -r dotenv/config ./src/server.js", dotenv was working with version 0.25.0

Im also on node version 14 but going to try latest of that as well in a bit.

No errors are showing up so I'm not sure where to start looking

Apollon77 commented 2 years ago

Hm ... I would not know where node-coap relies on env variables at all ... so honestly: Does it work without dotenv?

Else enable debug log and show whats happening ....

invaderb commented 2 years ago

Yeah I enabled debug log to see if I could see anything and nada

invaderb commented 2 years ago

Screen Shot 2022-03-02 at 2 01 47 PM

Apollon77 commented 2 years ago

And how it looks without dotenv?

invaderb commented 2 years ago

starts working just fine but local env vars don't get loaded :( Screen Shot 2022-03-02 at 2 04 44 PM

invaderb commented 2 years ago

Not sure if this is an issue with dotenv blocking the incoming requests but wouldn't make much sense as it's just loading the env vars into the process when node first starts up.

Apollon77 commented 2 years ago

@JKRhb any idea? I can not imagine that it should make any difference

JKRhb commented 2 years ago

Hmm :/ Maybe this is somehow caused by the switch to typescript?

invaderb commented 2 years ago

I think this might actually be an issue with the node project using ESModules, I finally got an error with dotenv running just a simple http server Screen Shot 2022-03-02 at 2 20 08 PM r

invaderb commented 2 years ago

well nvm I realized I has a spelling error >.< it did work with the node http module so back to square one

invaderb commented 2 years ago

@JKRhb are you able to reproduce for a sanity check?

invaderb commented 2 years ago

🤦 I have no idea why but I started removing my env vars one by and and I removed my server PORT var and it started working

invaderb commented 2 years ago

so something like this is not working

const port = process.env.S_PORT || 65011;
server.listen(port, () => {
    console.log(Date());
    console.log('\n=================================================\nCOAP LISTENER running at port ' +
    `${port}` + `\n Worker ${process.pid} is listening ` +
    '\n=================================================\n');
});
invaderb commented 2 years ago

ah ok figured it out, due to the migration to TS a check must has been added to ensure the port is a number.

This works

const port = parseInt(process.env.S_PORT) || 65011;
server.listen(port, () => {
    console.log(Date());
    console.log('\n=================================================\nCOAP LISTENER running at port ' +
    `${port}` + `\n Worker ${process.pid} is listening ` +
    '\n=================================================\n');
});
JKRhb commented 2 years ago

@JKRhb are you able to reproduce for a sanity check?

Oh, sorry for the late response! :/

ah ok figured it out, due to the migration to TS a check must has been added to ensure the port is a number.

Oh, awesome that you could figure it out 🎉 Would you consider this issue resolved then? Or do you think some kind of documentation should be added in case others run into the same problem?

invaderb commented 2 years ago

@JKRhb yeah I just think a quick little note next to the port in the readme would suffice! :) especially since pre 1.0.0 it could take in a string

JKRhb commented 2 years ago

@JKRhb yeah I just think a quick little note next to the port in the readme would suffice! :)

That sounds good :)

especially since pre 1.0.0 it could take in a string

I think the documentation in general could probably be updated – either in the README or maybe auto-generated using the TS typings and Github Actions, for example. The structure of the possible parameters for each method/constructor is pretty complex IMHO and not that well reflected at the moment.

...but I guess that is step we can take after documenting the new port parameter ;)

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within the next 7 days. Please check if the issue is still relevant in the most current version of the adapter and tell us. Also check that all relevant details, logs and reproduction steps are included and update them if needed. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within the next 7 days. Please check if the issue is still relevant in the most current version of the adapter and tell us. Also check that all relevant details, logs and reproduction steps are included and update them if needed. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically closed because of inactivity. Please open a new issue if still relevant and make sure to include all relevant details, logs and reproduction steps. Thank you for your contributions.