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

Fix undefined fileName in getFileName #68

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brandon-fry
Copy link

When attempting to find the file name of the caller, getFileName grabs a stack
trace and attempts to walk up it to find the first file name that is not the
bindings.js file. For some reason, in my environment, the bindings.js file
name is 'undefined' (see output below). In this case 'undefined' !== __filename
and the for loop is stopped, leaving fileName undefined and causing an error
when indexOf is invoked on fileName. To fix, an additional check for undefined
fileName is added in the loop.

fileName: undefined
function: getFileName
fileName: undefined
function: bindings
fileName: undefined
function: eval
fileName: /projects/node-crfpp/node-crfpp-bundle.js
function: ./node-crfpp.js

When attempting to find the file name of the caller, getFileName grabs a stack
trace and attempts to walk up it to find the first file name that is not the
bindings.js file. For some reason, in my environment, the bindings.js file
name is 'undefined' (see output below). In this case 'undefined' !== __filename
and the for loop is stopped, leaving fileName undefined and causing an error
when indexOf is invoked on fileName. To fix, an additional check for undefined
fileName is added in the loop.

fileName:  undefined
function:  getFileName
fileName:  undefined
function:  bindings
fileName:  undefined
function:  eval
fileName:  /projects/node-crfpp/node-crfpp-bundle.js
function:  ./node-crfpp.js
@brandon-fry
Copy link
Author

Potential fix for #54

@peec peec mentioned this pull request May 19, 2021
3 tasks
@eden-lane
Copy link

Any progress on this?

@nicolaichuk
Copy link

@TooTallNate Please review and merge to upstream.

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

Successfully merging this pull request may close these issues.

3 participants