Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError: winston.transports.Loggly is not a constructor #26

Closed
wants to merge 1 commit into from

Conversation

@mdemri
Copy link

@mdemri mdemri commented Oct 3, 2017

Taking winston 2.4.0 doesn't work, so don't

@mostlyjason
Copy link

@mostlyjason mostlyjason commented Oct 3, 2017

@mdemri can you offer more details about what error you found with Winston 2.4?

@mdemri
Copy link
Author

@mdemri mdemri commented Oct 3, 2017

Code:
const transports = [
new (winston.transports.Console)({
formatter(options) {
// Return string will be passed to logger.
return ${new Date()} ${options.level} ${options.message ? options.message : ''};
},
}),
];

if (condition) {
require('winston-loggly-bulk'); // eslint-disable-line
transports.push(
new (winston.transports.Loggly)(_.assign(config, {
isBulk: true,
tags: ['blah', 'blahblah'],
})));
}
const logger = new (winston.Logger)({
transports,
});

Output:
new (winston.transports.Loggly)(_.assign(config, {

TypeError: winston.transports.Loggly is not a constructor

@mostlyjason
Copy link

@mostlyjason mostlyjason commented Oct 3, 2017

Okay thanks we will have an engineer take a look at this in the next couple weeks

@mdemri
Copy link
Author

@mdemri mdemri commented Oct 3, 2017

Weeks?

It's an unprofessional behavior.
You're exposing your clients to bugs.

@mostlyjason
Copy link

@mostlyjason mostlyjason commented Oct 3, 2017

Sorry we'd like to test it to confirm the bug or at least confirm others are affected, especially since minor version updates are supposed to be backwards compatible. We run sprints and it's at the top of our backlog. There are a few issues ahead of this one. If you get to it before us, we'd be grateful for a PR that fixes the issue as well.

@Shwetajain148
Copy link

@Shwetajain148 Shwetajain148 commented Oct 16, 2017

@mdemri, I tried your sample code at my end to reproduce the issue mentioned by you but couldn't see any error. Can you please verify the below steps at your end to confirm that we both are following the same standard instructions for node logging.

  1. run the command "npm install winston-loggly-bulk" to setup the library.
  2. Create an app.js file with the code- app.txt
  3. Simply run your app.js file using command node app.js.

I tested the code with winston version 2.4.0 and 2.3.1, node-loggly-bulk version 2.0.1 and 2.0.2, winston-loggly-bulk version 2.0.1, node version 6.10.1 and everything worked fine for me, my event also reached to Loggly successfully.

FYI- The error TypeError: winston.transports.Loggly is not a constructor may also occur if the module winston-loggly-bulk couldn't be loaded in your project for any reason and is undefined.

Please try the above steps and see if this resolve your issue?

cc: @mostlyjason

@adambiggs
Copy link

@adambiggs adambiggs commented Oct 18, 2017

I'm also experiencing this error with winston 2.4.0 and winston-loggly-bulk 2.0.1 in node 8.7.0.

@mostlyjason
Copy link

@mostlyjason mostlyjason commented Oct 18, 2017

Thanks @adambiggs! @Shwetajain148 can you test these versions?

@Shwetajain148
Copy link

@Shwetajain148 Shwetajain148 commented Oct 23, 2017

@adambiggs I tested with winston 2.4.0, winston-loggly-bulk 2.0.1 in node 8.7.0 and 8.6.0 but couldn't see any error at my end. It would be great if you can offer your reproducible code to me so that I can look at the issue since I am not able to reproduce this error at my end.

cc: @mostlyjason

@adambiggs
Copy link

@adambiggs adambiggs commented Oct 24, 2017

Sorry @Shwetajain148, we've moved away from this package and I don't have time to look into it any further... But if it makes any difference I was using Yarn to install dependencies when I noticed the issue.

@Shwetajain148
Copy link

@Shwetajain148 Shwetajain148 commented Oct 26, 2017

@adambiggs @mdemri I have tried in the ways provided by you(including Yarn) but couldn't reproduce this error on my end. If you ever come across with the reproducible code for this error in future please share it with me with the environment details, I will be happy to fix the issue.

Thanks.

@mostlyjason
Copy link

@mostlyjason mostlyjason commented Nov 2, 2017

@jimdubbs in your travisci environment, can you check the package lock or logs to see if the modules for winston-loggly-bulk and the dependency node-loggly-bulk were loaded? Also, which node version is it running?

@mbesnili
Copy link

@mbesnili mbesnili commented Nov 11, 2017

This issue still occurs. I'm using node version 8.9.0, winston 2.4.0, winston-loggly-bulk 2.0.1

@mostlyjason
Copy link

@mostlyjason mostlyjason commented Nov 13, 2017

@mbesnili can you paste your error message? Is it "winston.transports.Loggly is not a constructor"? Also, can you paste the code you are using to construct the loggly transport?

@Rubinski78
Copy link

@Rubinski78 Rubinski78 commented Nov 28, 2017

Hi, i have the same error using the lib in node-red.
"TypeError: transport is not a constructor"

When i use the next statement it works
winston.add(winston.transports.Loggly, {
token: "mytoken",
subdomain: "mysubdomain",
tags: ["test"],
json:true
});

but i need the var to do a .remove at the end.

Is there a workaround?

this is my code:

var require = global.get('require');

var winston = require('winston');
require('winston-loggly-bulk');

var loggly = new winston.transports.Loggly({
token: "mytoken",
subdomain: "mysubdomain",
tags: ["test"],
json:true
});

winston.add(loggly);

winston.log('info',"Hello World from Node.js!");
winston.remove(loggly);
//winlog.flushLogsAndExit(); --> causes node-red to crash

return msg;

@Shwetajain148
Copy link

@Shwetajain148 Shwetajain148 commented Nov 29, 2017

Hi @mdemri @adambiggs @mbesnili can you please let me know the npm version you are running in your setup? It may possible that you are running an old version of npm which is causing this issue to you?

@Rubinski78
Copy link

@Rubinski78 Rubinski78 commented Nov 29, 2017

@Shwetajain148
Copy link

@Shwetajain148 Shwetajain148 commented Nov 30, 2017

@Rubinski78 I was able to reproduce the error reported by you if I follow your exact code. So what happening here is that if you are creating the Loggly transporter using the new keyword and storing it into a variable(to do .remove() later) then you are instantly creating an instance of it. Since the add method of winston may take 3 parameters like add(transport: TransportInstance, options?: TransportOptions, created?: boolean): LoggerInstance; you have to pass 2 more parameter while doing winston.add() like winston.add(loggly, null, true); to tell the winston that you have already created an instance of Loggly transporter. You can refer the below code line for more clarity.

Ref- https://github.com/winstonjs/winston/blob/2.4.0/lib/winston/logger.js#L481

Please follow the below code that should works for you and if not then let me know.

const winston = require('winston');
var winlog = require('winston-loggly-bulk');

var loggly = new winston.transports.Loggly({
    token: "loggly-token",
    subdomain: "loggly-subdomain",
    tags: ["test"],
    json: true
});

winston.add(loggly, null, true);

winston.log('info', "Hello World from Node.js!");
winston.remove(loggly);
winlog.flushLogsAndExit();

Also, please note that if you are using the flushLogsAndExit() method of winston-loggly-bulk then you have to do something like var winlog = require('winston-loggly-bulk'); and then you will be able to do winlog.flushLogsAndExit();.

@mostlyjason mostlyjason changed the title don't take latest version of winston TypeError: winston.transports.Loggly is not a constructor Dec 6, 2017
@mostlyjason
Copy link

@mostlyjason mostlyjason commented Dec 14, 2017

Hey guys we have updated our documentation to explain using the new keyword, and we've also created a troubleshooting step addressing the error message: https://www.loggly.com/docs/node-js-logs/. Since we haven't seen any additional reports since describing the fix, I'm going to close this issue for now. Please reopen it if you experience the issue after implementing Shweta's solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.