github icon indicating copy to clipboard operation
github copied to clipboard

Repository.writeFile fails if no options object is passed

Open lantchou opened this issue 6 years ago • 3 comments

In the documentation about Repository.writeFile, the options parameter is specified as optional. Despite this, the function fails if no options object is passed and throws this error:

TypeError: Cannot read property 'encode' of undefined
    at Repository.writeFile (/Users/lucasantchougov/Documents/Work/oauth-rule-manager/node_modules/github-api/dist/components/Repository.js:941:37)
    at GitHubRuleManager.commitWorkspace (/Users/lucasantchougov/Documents/Work/oauth-rule-manager/lib/rule_managers/github-rule-manager.js:143:18)
    at /Users/lucasantchougov/Documents/Work/matey/matey-server/app.js:74:25
    at Layer.handle [as handle_request] (/Users/lucasantchougov/Documents/Work/matey/matey-server/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/lucasantchougov/Documents/Work/matey/matey-server/node_modules/express/lib/router/route.js:137:13)
    at Route.dispatch (/Users/lucasantchougov/Documents/Work/matey/matey-server/node_modules/express/lib/router/route.js:112:3)
    at Layer.handle [as handle_request] (/Users/lucasantchougov/Documents/Work/matey/matey-server/node_modules/express/lib/router/layer.js:95:5)
    at /Users/lucasantchougov/Documents/Work/matey/matey-server/node_modules/express/lib/router/index.js:281:22
    at param (/Users/lucasantchougov/Documents/Work/matey/matey-server/node_modules/express/lib/router/index.js:354:14)
    at param (/Users/lucasantchougov/Documents/Work/matey/matey-server/node_modules/express/lib/router/index.js:365:14)

The error seems to get thrown when options.encode is read in Repository.writeFile. It only works for me when passing an empty object. This should be better documented or a default empty object should be instantiated in the method.

lantchou avatar Aug 09 '19 10:08 lantchou

It seems to work without having the options parameter when the callback function is used, but ideally you should be able to call writeFile without either the options parameter or the callback function. I will look into this.

CodyGramlich avatar Sep 21 '19 18:09 CodyGramlich

I just realized this is solved in PR #588.

CodyGramlich avatar Sep 21 '19 18:09 CodyGramlich

After looking at this a little more closely, it looks like our old approach was to require a callback. If a function was provided as the options param, we would assume that the user was passing in the callback function. When we added promises, people stopped using callbacks, so we can no longer assume that this is a required parameter (even though the docs say it is).

j-rewerts avatar Oct 08 '19 12:10 j-rewerts