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

Duplicate "Setting artifact for function" logs (possible redundant calls to setArtifactPath?) #1242

Closed
NoxHarmonium opened this issue Sep 15, 2022 · 2 comments · Fixed by #1245
Labels
Milestone

Comments

@NoxHarmonium
Copy link
Contributor

Duplicate "Setting artifact for function" logs (possible redundant calls to setArtifactPath?)

Description

I guess this might be a minor bug but I did spend a fair chunk of time debugging it so I think it is worth raising. If the issue is valid I'll be happy to raise a PR to get a fix in.

What went wrong?

Every time I package or deploy with the --verbose switch I get duplicate "Setting artifact for function" messages.

For example with individual packaging enabled and a function called "myFunction"

yarn sls package --stage sandbox --verbose
...
[Webpack] Copying existing artifacts
Setting artifact for function 'myFunction' to '.serverless/myFunction.zip'
Setting artifact for function 'myFunction2' to '.serverless/myFunction2.zip'
Setting artifact for function 'myFunction' to '.serverless/myFunction.zip'
Setting artifact for function 'myFunction2' to '.serverless/myFunction2.zip'
...

What did you expect should have happened?

Not to have duplicate logs for each function

What was the config you used?

import type { AWS } from '@serverless/typescript';
import myFunction from '@functions/myFunction';
import myFunction2 from '@functions/myFunction2';

const serverlessConfiguration: AWS = {
  service: 'example-project',
  frameworkVersion: '3',
  plugins: ['serverless-webpack'],
  provider: {
    name: 'aws',
    region: 'ap-southeast-2',
    runtime: 'nodejs14.x',
    stage: '${opt:stage}',
    apiGateway: {
      minimumCompressionSize: 1024,
      shouldStartNameWithService: true,
      apiKeys: ['foo-bar'],
    },
    environment: {
      AWS_NODEJS_CONNECTION_REUSE_ENABLED: '1',
      NODE_OPTIONS: '--enable-source-maps --stack-trace-limit=1000',
    },
  },
  // import the function via paths
  functions: {
    myFunction,
    myFunction2,
  },
  package: { individually: true },
  resources: {
    Resources: {},
  },
  custom: {
    webpack: {
      packager: 'yarn',
      includeModules: {
        forceExclude: ['some-lib'],
      },
    },
  },
};

module.exports = serverlessConfiguration;

What stacktrace or error message from your provider did you see?

No explicit stack trace or error message

Suggested Fix

I could be wrong but it looks like there might be some duplicate code in lib/packageModules.js

First there is a loop to call setArtifactPath for each function.

_.forEach(functionNames, funcName => {

it already has logic to set the correct artefact path depending on whether individual packaging is enabled or not

const archiveName = isIndividualPackaging.call(this) ? funcName : this.serverless.service.getServiceObject().name;

Then further down, if individual packaging is enabled, setArtifactPath is called again with the same args which I think is redundant

_.forEach(functionNames, funcName => {

If I understand correctlty, I think the following patch might do the trick, without introducing any side effects. It is very rough, and I haven't tested it. If you think my analysis is correct I'll go over it again and make sure tests pass etc.

See what you think:

---
 lib/packageModules.js | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/lib/packageModules.js b/lib/packageModules.js
index fce5493..e794b29 100644
--- a/lib/packageModules.js
+++ b/lib/packageModules.js
@@ -224,42 +224,33 @@ module.exports = {
     // only function passed with `-f` flag should be processed.
     const functionNames = this.options.function ? [this.options.function] : getAllNodeFunctions.call(this);
 
+    const serviceName = this.serverless.service.getServiceObject().name;
+
     // Copy artifacts to package location
     if (isIndividualPackaging.call(this)) {
       _.forEach(functionNames, funcName => copyArtifactByName.call(this, funcName));
     } else {
       // Copy service packaged artifact
-      const serviceName = this.serverless.service.getServiceObject().name;
       copyArtifactByName.call(this, serviceName);
     }
 
     _.forEach(functionNames, funcName => {
       const func = this.serverless.service.getFunction(funcName);
 
-      const archiveName = isIndividualPackaging.call(this) ? funcName : this.serverless.service.getServiceObject().name;
+      const archiveName = isIndividualPackaging.call(this) ? funcName : serviceName;
 
       const { serverlessArtifact } = getArtifactLocations.call(this, archiveName);
       setArtifactPath.call(this, funcName, func, serverlessArtifact);
     });
 
     // Set artifact locations
-    if (isIndividualPackaging.call(this)) {
-      _.forEach(functionNames, funcName => {
-        const func = this.serverless.service.getFunction(funcName);
-
-        const archiveName = funcName;
-
-        const { serverlessArtifact } = getArtifactLocations.call(this, archiveName);
-        setArtifactPath.call(this, funcName, func, serverlessArtifact);
-      });
-    } else {
-      const archiveName = this.serverless.service.getServiceObject().name;
+    if (!isIndividualPackaging.call(this) && 
+        _.get(this.serverless, 'service.provider.name') === 'google') {
+      const archiveName = serviceName;
 
       const { serverlessArtifact } = getArtifactLocations.call(this, archiveName);
 
-      if (_.get(this.serverless, 'service.provider.name') === 'google') {
-        setServiceArtifactPath.call(this, serverlessArtifact);
-      }
+      setServiceArtifactPath.call(this, serverlessArtifact);
     }
 
     return BbPromise.resolve();
-- 

Additional Data

  • Serverless-Webpack Version you're using:
    5.7.1
  • Webpack version you're using:
    5.73.0
  • Serverless Framework Version you're using:
    Framework Core: 3.7.1 (local)
    Plugin: 6.1.5
    SDK: 4.3.2
  • Operating System:
    MacOS Monterey 12.6

Also

Thanks for the great plugin!

@j0k3r j0k3r added the bug label Sep 15, 2022
@j0k3r
Copy link
Member

j0k3r commented Sep 15, 2022

That's seems to be a valid fix, you can open a PR.
Thanks for the clear explanation!

@NoxHarmonium
Copy link
Contributor Author

No worries! I should have a PR up within the next few days

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

Successfully merging a pull request may close this issue.

2 participants