yagop / node-telegram-bot-api

Telegram Bot API for NodeJS
MIT License
8.46k stars 1.53k forks source link

bot.sendMessage echos the amount chatId is activated(?) #708

Closed RFlintstone closed 5 years ago

RFlintstone commented 5 years ago

When I try to execute a command with bot.onText and then use bot.sendMessage(chatId, "Message");

It will count all messages due to chatId but only echo when the right command is executed.

Ex:

1)
User: /echo Hi 
Bot: Hi

2)
User: /echo Hi 
Bot: Hi
Bot: Hi

3) 
User: just some text

4)
User: /echo Hi 
Bot: Hi
Bot: Hi
Bot: Hi
Bot: Hi

My code: https://hastebin.com/kojezuxete.js

alexandercerutti commented 5 years ago

I suppose this is related to #700 (you moved to a new issue).

As I said in the other issue, you have to separate the two actions. Currently, you are doing this:

0) Bot creates a new listener to generic "message";
1) User: /echo Hi
    - bot receives any message with "message" listener
    - Creates a new Listener for message "/echo (.+)";
    - Detects the message with this new listener
    - Fires new listener callback

2) User: /echo Hi
    - bot receives any message with "message" listener
    - Creates a new Listener for message "/echo (.+)";
    - Detects the message with this new listener
    - Fires new listener callback
    - Older listener for message "/echo (.+)" executes.

...

so you should do this:

bot.on('message', (msg) => {
    const chatId = msg.chat.id;

    if (msg.text == "/start") {
        if (standardusername == "Guest") {
            bot.sendMessage(chatId, "Sumbit your name with /name {name}")
        }
    }
});

bot.onText(/\/echo (.+)/, (msg, match) => {
       // 'msg' is the received Message from Telegram
        // 'match' is the result of executing the regexp above on the text content
        // of the message

        const resp = match[1]; // the captured "whatever"

        // send back the matched "whatever" to the chat
        bot.sendMessage(chatId, resp);
});

But again, .on("message") should be used to detect a generic message, not a specific message. So, for /start, you should do the same of /echo.

RFlintstone commented 5 years ago
    const chatId = msg.chat.id;

@alexandercerutti - Okay, and if I want to seperate modules/commands? How do I do that? I currently have this: Index.js: https://hastebin.com/edibosujep.js name.js: https://hastebin.com/huziwubiqu.php

alexandercerutti commented 5 years ago

You have to create a "core" module that you will import in every module and then an index.js that will load all these modules.

Like this:

// core.js

const TelegramBot = require("node-telegram-bot-api");
const bot = new TelegramBot(<token />, { polling: true });

module.exports = bot; 
// module1.js

const bot = require("./core.js");
bot.onText(/\/example/, (msg, match) => { ... });
// index.js

const module1 = require("./module1.js");
RFlintstone commented 5 years ago

You have to create a "core" module that you will import in every module and then an index.js that will load all these modules.

Like this:

// core.js

const TelegramBot = require("node-telegram-bot-api");
const bot = new TelegramBot(<token />, { polling: true });

module.exports = bot; 
// module1.js

const bot = require("./core.js");
bot.onText(/\/example/, (msg, match) => { ... });
// index.js

const module1 = require("./module1.js");

Hmm, so something like this? Index.js: https://hastebin.com/foziwohoto.js Name.js: https://hastebin.com/nenidajaya.php Core.js: https://hastebin.com/soxehodati.java

(And then run the core.js file?)

alexandercerutti commented 5 years ago

Yes, about. you don't have to export a function as if you call it, you will end up again in the same situation as above. Since you are exporting a bot from the core, if you import in your module, you will have it as "global" variable. Also, you are not required to export something from a module: it may just have a side-effect.

I committed an error in the example above: in index.js I load module1.js as a module but you might just load it as a side-effect: I mean, you just use the file to load the bot behavior, like this:

require("./module1.js");
require("./module2.js");

No, don't run core.js: if you run only core.js, you won't load the listener registration and your bot won't react to any message (try it).

RFlintstone commented 5 years ago

Yes, about. you don't have to export a function as if you call it, you will end up again in the same situation as above. Since you are exporting a bot from the core, if you import in your module, you will have it as "global" variable. Also, you are not required to export something from a module: it may just have a side-effect.

I committed an error in the example above: in index.js I load module1.js as a module but you might just load it as a side-effect: I mean, you just use the file to load the bot behavior, like this:

require("./module1.js");
require("./module2.js");

No, don't run core.js: if you run only core.js, you won't load the listener registration and your bot won't react to any message (try it).

Ok, so I did change one line of code (in index.js) and that loaded the bot (I run index.js, as you said) however it doesn't seem to find a module (needed to add require core.js otherwhise the bot didn't want to start)

Added line:

//Modules
    //Command Requires
    var commandName = require('./commands/name.js');
    Added this line >> const bot = require("./core.js");

    //Command Calls
    bot.on('message', (msg) => {
        const chatId = msg.chat.id;

        commandName.Name();

        //Echo message in terminal
        console.log(standardusername + ': ' + msg.text);
    });

But on every message it says error: [polling_error] {"code":"MODULE_NOT_FOUND"}. So my guess it that it can't find core.js for some reason.

alexandercerutti commented 5 years ago

Yeah okay, so in index.js you are loading core.js. Fine, but did you change also name.js structure to not export a function?

RFlintstone commented 5 years ago

Yeah okay, so in index.js you are loading core.js. Fine, but did you change also name.js structure to not export a function?

No, I didn't. When I disable that (name.js) it trows an error/doesn't want to startup.

"C:\Program Files\nodejs\node.exe" index.js
internal/modules/cjs/loader.js:583
    throw err;
    ^

Error: Cannot find module './core.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:581:15)
    at Function.Module._load (internal/modules/cjs/loader.js:507:25)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\commands\name.js:2:17)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\index.js:40:23)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)

Process finished with exit code 1
alexandercerutti commented 5 years ago

Where is core.js file located respect index.js? You know that "." means "current directory", don't you?

RFlintstone commented 5 years ago

Where is core.js file located respect index.js? You know that "." means "current directory", don't you?

I do. image

alexandercerutti commented 5 years ago

That's weird. How do you start the bot? Anyway, make requiring all needed modules at the top a habit, just because it is more readable.

RFlintstone commented 5 years ago

That's weird. How do you start the bot? Anyway, make requiring all needed modules at the top a habit, just because it is more readable.

Image of my configuration. It worked when I had everything in one file (but that isn't very readable). image

alexandercerutti commented 5 years ago

I don't know which is the "current dir" if you start on windows. Try to print above all "dirname" ( `console.log(dirname)` ).

RFlintstone commented 5 years ago

I don't know which is the "current dir" if you start on windows. Try to print above all "dirname" ( `console.log(dirname)` ).

I'm afraid it doesn't know __dirname (console.log(__dirname))

alexandercerutti commented 5 years ago

How the hell is this even possible?! __dirname is a global variable of Node.js...

RFlintstone commented 5 years ago

How the hell is this even possible?! __dirname is a global variable of Node.js...

That's what I try to figure out. I use v10.14.2 of node.js (i'm sure that's one of the latest (stable) builds) Does it have something to do with the IDE? What do you use? Intelij? PHPStorm? Atom?

RFlintstone commented 5 years ago

How the hell is this even possible?! __dirname is a global variable of Node.js...

Ok, so, good news, I did some digging and it now 'knows' __dirname. This is the output:

Dirname: C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\commands

(console.log('Dirname: ' + __dirname); in name.js)

In index.js the output is the following:

Dirname: C:\Users\rwfli\Documents\Projects\UltiConBot\Version2

EDIT: It looks like it only uses the require from name.js (that is in a subfolder) so I did ../core.js and that seems to fix the require issue.

RFlintstone commented 5 years ago

@alexandercerutti - So the problem is basicaly fixed but it doesn't want to change the name?

console.log('Dirname: ' + __dirname);

    const bot = require('../core.js');
    var config = require('../config.json');
    var standardusername = config.information.normalusername;

    bot.onText(/\/name (.+)/, (msg, match) => {
        const chatId = msg.chat.id;
        // 'msg' is the received Message from Telegram
        // 'match' is the result of executing the regexp above on the text content
        // of the message

        const name = match[1]; // the captured name

        bot.sendMessage(chatId, 'Welcome ' + name + '!');
        config.information.normalusername = name;

    });
alexandercerutti commented 5 years ago

Obviously, you have to go back if your core.js is in the parent folder, eheh. 😁 Okay, so you are trying to read the name and save it in the config.json, aren't you?

RFlintstone commented 5 years ago

Obviously, you have to go back if your core.js is in the parent folder, eheh. 😁 Okay, so you are trying to read the name and save it in the config.json, aren't you?

I am trying that, yes, but only for one user. (Telegram username/firstname is also fine)

alexandercerutti commented 5 years ago

Only for a user... mmm. The fact here is that the function is available to all the users that calls the method. Anyway, that is not how it works. When you do require("file.json"), you are simply loading the text stream in memory. So, if you call config.information.normalusername in the same file after assigning, you'll see the difference, but you are not storing any changes on the disk yet. What you have to do, is to read it (and you are already doing that), then once edited, make it a string (with JSON.stringify(...)) and write the new content to the disk. Does it make sense?

To write it on the disk, use this method: fs.writeFile

RFlintstone commented 5 years ago

Only for a user... mmm. The fact here is that the function is available to all the users that calls the method. Anyway, that is not how it works. When you do require("file.json"), you are simply loading the text stream in memory. So, if you call config.information.normalusername in the same file after assigning, you'll see the difference, but you are not storing any changes on the disk yet. What you have to do, is to read it (and you are already doing that), then once edited, make it a string (with JSON.stringify(...)) and write the new content to the disk. Does it make sense?

To write it on the disk, use this method: fs.writeFile

@alexandercerutti - I decided to go with the database route however it doesn't seem to work with msg.text.username.toString(). And due to the fact this isn't a string this says it's 'null'. Do you know how to fix this?

Code + Result/Console output: https://hastebin.com/hacerutezi.http

alexandercerutti commented 5 years ago

Because msg.text is not an object but just a string and it is the real content the user sent to your bot. To get the username, you should refer to msg.from.username o msg.chat.username, but please notice that you should check if it exists before: a user might not have a username. For any doubts about where to gather infos, look at the official bot api documentation

RFlintstone commented 5 years ago

Because msg.text is not an object but just a string and it is the real content the user sent to your bot. To get the username, you should refer to msg.from.username o msg.chat.username, but please notice that you should check if it exists before: a user might not have a username. For any doubts about where to gather infos, look at the official bot api documentation

So something like this?

    if(msg.from.username != ''){
        username = msg.from.username;
    }
alexandercerutti commented 5 years ago

Yes, exactly. Use always !==.

RFlintstone commented 5 years ago

Yes, exactly. Use always !==.

It kinda worked! It creates a name in the database however they are both zeros. Any input on this?

https://hastebin.com/kesejitewe.http

error: [polling_error] {}
RFlintstone aka Guest: /name Ruben
INSERT INTO nickname_data VALUES (`username` = 'RFlintstone', `nickname` = 'Ruben')
OkPacket {
  fieldCount: 0,
  affectedRows: 1,
  insertId: 0,
  serverStatus: 2,
  warningCount: 0,
  message: '',
  protocol41: true,
  changedRows: 0 }
alexandercerutti commented 5 years ago

I don't understand, what is 0?

RFlintstone commented 5 years ago

I don't understand, what is 0?

I think it expects something else. According to the output it haves something todo with the polling error.

alexandercerutti commented 5 years ago

the polling error may mean anything. Insert in your bot this chunk. It will give you more details:

bot.on("polling_error", (msg) => console.log(msg));
RFlintstone commented 5 years ago
Dirname: C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\commands
Sun, 24 Mar 2019 20:41:14 GMT node-telegram-bot-api deprecated Automatic enabling of cancellation of promises is deprecated.
In the future, you will have to enable it yourself.
See https://github.com/yagop/node-telegram-bot-api/issues/319. at internal\modules\cjs\loader.js:689:30
Connected!
App name: ultimatecontroller
RFlintstone aka Guest: /name Ruben
ReferenceError: err is not defined
    at Object.bot.onText [as callback] (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\commands\name.js:48:5)
    at _textRegexpCallbacks.some.reg (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\node-telegram-bot-api\src\telegram.js:610:15)
    at Array.some (<anonymous>)
    at TelegramBot.processUpdate (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\node-telegram-bot-api\src\telegram.js:601:35)
    at updates.forEach.update (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\node-telegram-bot-api\src\telegramPolling.js:110:22)
    at Array.forEach (<anonymous>)
    at _lastRequest._getUpdates.then.updates (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\node-telegram-bot-api\src\telegramPolling.js:106:17)
    at tryCatcher (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\bluebird\js\release\util.js:16:23)
    at Promise._settlePromiseFromHandler (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\bluebird\js\release\promise.js:512:31)
    at Promise._settlePromise (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\bluebird\js\release\promise.js:569:18)
    at Promise._settlePromise0 (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\bluebird\js\release\promise.js:614:10)
    at Promise._settlePromises (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\bluebird\js\release\promise.js:694:18)
    at _drainQueueStep (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\bluebird\js\release\async.js:138:12)
    at _drainQueue (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\bluebird\js\release\async.js:131:9)
    at Async._drainQueues (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\bluebird\js\release\async.js:147:5)
    at Immediate.Async.drainQueues [as _onImmediate] (C:\Users\rwfli\Documents\Projects\UltiConBot\Version2\node_modules\bluebird\js\release\async.js:17:14)
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)
INSERT INTO nickname_data VALUES (`username` = 'RFlintstone', `nickname` = 'Ruben')
OkPacket {
  fieldCount: 0,
  affectedRows: 1,
  insertId: 0,
  serverStatus: 2,
  warningCount: 0,
  message: '',
  protocol41: true,
  changedRows: 0 }
alexandercerutti commented 5 years ago

So, here is your problem. err is not defined in the onText callback in name:48

RFlintstone commented 5 years ago

Ok, that is solved. The query still outputs 0, 0 though

    var setnickname = {
      username_: username,
      nickname_: name
    };

    var sql = con.query('INSERT INTO nickname_data VALUES (?)', setnickname, function (err, result){
        console.log(sql.sql);
        if (err) {
            console.error(err)
            return;
        }
        console.error(result);
    });

image

alexandercerutti commented 5 years ago

I don't know, I cannot help you with this and this is not the right place to talk about this. Since this has been solved, please, close the issue topic. 😉

RFlintstone commented 5 years ago

I don't know, I cannot help you with this and this is not the right place to talk about this. Since this has been solved, please, close the issue topic. 😉

Will do! Thanks for the great help you have given me today! :)

alexandercerutti commented 5 years ago

You are welcome 😊