Skip to content
This repository was archived by the owner on Dec 9, 2024. It is now read-only.

Conversation

shazron
Copy link
Contributor

@shazron shazron commented Oct 5, 2018

Fixes #138

In the base class base.js function getArtifactZip, we already read the zipfile into a buffer

return this.getArtifactZip(functionObject)
. This is then passed to the subclass' processActionPackage function, in this case in java.js
processActionPackage(handlerFile, zip) {
, which thinks the zip parameter is a JSZip object, thus it all fails with a NPE since zip.file does not evaluate. We just need toreturn zip in the function.

On a cursory review, I think other runtimes may make this mistake as well I think: https://github.com/serverless/serverless-openwhisk/search?q=processActionPackage&unscoped_q=processActionPackage

@csantanapr csantanapr requested a review from jthomas October 5, 2018 16:09
@codecov-io
Copy link

Codecov Report

Merging #139 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #139      +/-   ##
==========================================
- Coverage   83.56%   83.54%   -0.02%     
==========================================
  Files          46       46              
  Lines        1649     1647       -2     
  Branches      332      332              
==========================================
- Hits         1378     1376       -2     
  Misses        271      271
Impacted Files Coverage Δ
compile/functions/runtimes/java.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e4d641...429344e. Read the comment docs.

@jthomas jthomas merged commit 8eb2d2f into serverless:master Oct 8, 2018
@shazron shazron deleted the javafix branch October 9, 2018 11:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants