Windows paths

[philnash]

@devinrader was having difficulties deploying a function within a subdirectory on Windows. Ultimately the error message was:

{"message":"Path provided is invalid.","code":20001,"user_error":true,"http_status_code":400,"params":{}}' }

The issue was a function in $PROJECT_DIR/functions/sms/reply.js and the error was:

Error: Failed to upload Function /sms\reply

With an amount of VSCode live share and messing around, changing this line from functions.ts:

form.append('Path', fn.path);

to

form.append('Path', fn.path.replace(/\\/g, '/'));

got the deploy to work. The output still showed:

Functions:
   https://fancy-domain-3313-dev.twil.io/sms\foo

I stepped back to see where the path was coming from and I traced it back to getPathAndAccessFromFileInfo. upath was already in the dependency tree, so I promoted it to direct dependency and used toUnix to turn the eventual path of both functions and assets into something the API will understand.

This was painful working through the issue with a windows machine an ocean and a continent away, so I am not 100% confident that this does now work for all machines. I recommend testing against deployments on Mac and Windows with functions and assets in subfolders to ensure this is now working cross platform.

Thoughts?

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

• I acknowledge that all my contributions will be made under the project's license.

[dkundel]

Oh that’s a good catch. Thanks for jumping on this. From reading the lib on my phone, this seems fine. I wonder if we should do a change here as well:

serverless-api/src/utils/fs.ts

Line 113 in 7f5f7bc

const name = path.relative(dir, filePath);

That's largely used for naming things. Stefan's serverless framework integration for example lets you set the name explicitly.

I think we should turn it into UNIX because there might otherwise be issues because the name is used during deployment to look up the right Function/Asset to update. So two devs working on different platforms could have an issue.

[dkundel]

Example for Functions (same happens in Assets)

serverless-api/src/api/functions.ts

Line 86 in 7f5f7bc

const existingFn = existingFunctions.find(f => fn.name === f.friendly_name);

[dkundel]

Actually never mind my comment before. This looks good. I'll test it on Mac because it should at least not break existing behavior and then release a private build that I'll let Devin test.

[philnash]

I think you might be right from your comment before though... Let's test, but normalising the names and paths across OSes does seem important overall.

[dkundel]

Yes but I realized afterwards that the name gets actually overwritten by the path here:

serverless-api/src/utils/fs.ts

Line 246 in 7f5f7bc

name: path,

[philnash]

If we have a name and a path variable but one writes over the other, should we consolidate it to just the one of them?