${file(filename)} Syntax does not work for functions

[securityvoid]

This is a Bug Report

Attempting to include another YML or JSON file from within the "functions" declaration fails to load with the error:

Cannot assign to read only property 'events' of $

This traces back to the following section of the "load" routine in Service.js

// setup function.name property
        const stageNameForFunction = options.stage || this.provider.stage;
        _.forEach(that.functions, (functionObj, functionName) => {
          if (!functionObj.events) {
            that.functions[functionName].events = [];
          }
          if (!_.isArray(functionObj.events)) {
            throw new SError(`Events for "${functionName}" must be an array,` +
                             ` not an ${typeof functionObj.events}`);
          }

          if (!functionObj.name) {
            that.functions[functionName].name =
              `${that.service}-${stageNameForFunction}-${functionName}`;
          }
        });

Stepping through the code shows that at the time the above code is run, the variables have not been expanded yet. As a result the text is treated like literal text instead of having the file contents. This is what throws the error since none of the function attributes are set.

Commenting out the whole above block appears to run properly with no immediately foreseeable issues, but I'm not sure how critical the defaulting of the name attribute is on the function. Clearly the above code was added for a reason, even if I can't see it.

If the above code is not necissary, it could be removed to solve this issue. Alternatively, if the setting of the name attribute happens instead in the Run() method of Serverless.js after the variable substitution happens (Looks like this currently happens on line 85 of Serverless.js), this could work.

Additional Data

• Serverless Framework Version you're using: 1.4.0
• Operating System: Windows 10 Pro
• Stack Trace: TypeError: Cannot assign to read only property 'events' of $
  at C:\Users%USER%\AppData\Roaming\npm\node_modules\serverless\lib\classes\Service.js:160:49
  at index (C:\Users%USER%\AppData\Roaming\npm\node_modules\serverless\node_modules\lodash\lodash.js:4946:15)
  at Function.forEach (C:\Users%USER%\AppData\Roaming\npm\node_modules\serverless\node_modules\lodash\lodash.js:9344:14)
  at C:\Users%USER%\AppData\Roaming\npm\node_modules\serverless\lib\classes\Service.js:158:11
• Provider Error messages: Cannot assign to read only property 'events' of $

[securityvoid]

I don't know why my searching didn't pull this up, but this is definitely related to Issue #2418 .

[fruffin]

Pull request #2434 from a while back should fix this

[dannycohn]

@securityvoid @fruffin #2418 and #2434 didn't fix this issue. That fixed the ability to externalize just the events. This issue is that you cant externalize functions as a whole. It's due to this block of code in Service.js

const stageNameForFunction = options.stage || this.provider.stage;
        _.forEach(that.functions, (functionObj, functionName) => {
          if (!functionObj.events) {
            that.functions[functionName].events = [];
          }

          if (!functionObj.name) {
            that.functions[functionName].name =
              `${that.service}-${stageNameForFunction}-${functionName}`;
          }
        });

It's trying to make sure that each function has an array of events, even if empty, and also assigns a name to each function if not already populated. If this.functions is just the ${file(...)} string that hasn't been expanded yet, it ends up iterating over the characters of that string. I'm still looking to see if I can put together a PR to fix this, other than removing that block of code alltogether