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

Spurious error when providing local path to icons #251

Closed
conman124 opened this issue Jul 7, 2020 · 9 comments
Closed

Spurious error when providing local path to icons #251

conman124 opened this issue Jul 7, 2020 · 9 comments

Comments

@conman124
Copy link

I'm submitting a...

Not sure if this is a bug or if I'm requesting a new feature


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[x] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

When I run ask deploy when skill.json includes a smallIconUri with a local path (file://assets/blah.png), it completes successfully, and the icon gets uploaded correctly when viewing the skill in the web console, but I get the following errors:

[Error]: {
  "message": "Skill manifest is not valid.",
  "violations": [
    {
      "code": "INVALID_URL_FORMAT",
      "message": "String instance with value \"file://assets/en-US_smallIcon.png\" at property path \"$.manifest.publishingInformation.locales.en-US.smallIconUri\" is not a valid URL.",
      "validationDetails": {
        "originalInstance": {
          "dataType": "string",
          "propertyPath": "$.manifest.publishingInformation.locales.en-US.smallIconUri",
          "type": "BODY",
          "value": "file://assets/en-US_smallIcon.png"
        }
      }
    },
    {
      "code": "INVALID_URL_FORMAT",
      "message": "String instance with value \"file://assets/en-US_largeIcon.png\" at property path \"$.manifest.publishingInformation.locales.en-US.largeIconUri\" is not a valid URL.",
      "validationDetails": {
        "originalInstance": {
          "dataType": "string",
          "propertyPath": "$.manifest.publishingInformation.locales.en-US.largeIconUri",
          "type": "BODY",
          "value": "file://assets/en-US_largeIcon.png"
        }
      }
    }
  ]
}

Expected Behavior

There should be no error if the file exists. If the files do not exist, an error would be expected.

Current Behavior

There is an error even though the icon exists and gets properly uploaded to the web console.

Steps to Reproduce (for bugs)

skill.json:

{
  "manifest": {
    "publishingInformation": {
      "locales": {
        "en-US": {
          "summary": "TODO",
          "examplePhrases": [
            "fake"
          ],
          "name": "fake",
          "description": "TODO",
          "largeIconUri": "file://assets/en-US_largeIcon.png",
          "smallIconUri": "file://assets/en-US_smallIcon.png"
        }
      },
      "isAvailableWorldwide": false,
      "testingInstructions": "TODO",
      "category": "TO_DO_LISTS_AND_NOTES",
      "distributionCountries": [
        "US"
      ]
    },
    "apis": {
      "custom": {
        "endpoint": {
        }
      }
    },
    "manifestVersion": "1.0"
  }
}

Possible Solution

Not sure if it's an error on the command line side or smapi. It seems like it might be from smapi because I can't seem to find anything in ask-cli about it.

Your Environment and Context

  • ask-cli version: 2.8.0 (my apologies if this is fixed in a newer version, haven't seen anything about it in release notes)
  • Operating System and version: ubuntu 20.04
  • Node.js version used for development: 10.19.0
  • NPM version used for development: 6.14.4
@kakhaUrigashvili
Copy link
Contributor

kakhaUrigashvili commented Jul 7, 2020

@conman124 try putting images in the skill-package/assets/images folder as specified in the skill package format:
https://developer.amazon.com/en-US/docs/alexa/smapi/skill-package-api-reference.html#skill-package-format

and using a url similar to: file://assets/images/en-US_smallIcon.png

@kakhaUrigashvili kakhaUrigashvili added the Status: Wait Response Will close the issue in 7 days if no response for the thread label Jul 7, 2020
@conman124
Copy link
Author

Got the same error:

[Error]: {
  "message": "Skill manifest is not valid.",
  "violations": [
    {
      "code": "INVALID_URL_FORMAT",
      "message": "String instance with value \"file://assets/images/en-US_smallIcon.png\" at property path \"$.manifest.publishingInformation.locales.en-US.smallIconUri\" is not a valid URL.",
      "validationDetails": {
        "originalInstance": {
          "dataType": "string",
          "propertyPath": "$.manifest.publishingInformation.locales.en-US.smallIconUri",
          "type": "BODY",
          "value": "file://assets/images/en-US_smallIcon.png"
        }
      }
    },
    {
      "code": "INVALID_URL_FORMAT",
      "message": "String instance with value \"file://assets/images/en-US_largeIcon.png\" at property path \"$.manifest.publishingInformation.locales.en-US.largeIconUri\" is not a valid URL.",
      "validationDetails": {
        "originalInstance": {
          "dataType": "string",
          "propertyPath": "$.manifest.publishingInformation.locales.en-US.largeIconUri",
          "type": "BODY",
          "value": "file://assets/images/en-US_largeIcon.png"
        }
      }
    }
  ]
}

I'm willing to try any other suggestions.

@kakhaUrigashvili

This comment has been minimized.

@kakhaUrigashvili
Copy link
Contributor

kakhaUrigashvili commented Jul 8, 2020

@conman124
Also based on the instructions from https://developer.amazon.com/en-US/docs/alexa/smapi/skill-package-api-reference.html#skill-package-format
You may able to script this process. For example:

  1. ask smapi create-upload-url
  2. create zip file
  3. upload zip file to url from step 1
  4. ask smapi import-skill-package --location url-from-step-1 --skill-id your-skill-id

@conman124
Copy link
Author

conman124 commented Jul 12, 2020

@kakhaUrigashvili I would prefer to only use ask deploy if I can, and I think ask-cli should support this use case (since icons are required for skill validation). I have looked into it, and I think I have figured out the cause of the error message:

  1. ask-cli zips the skill package and uploads/imports it
  2. (this is my assumption) SMAPI puts the image assets into an S3 bucket and updates the manifest with valid URIs. I have verified that the manifest returned from ask smapi get-skill-manifest has the image assets pointing to an S3 URL.
  3. ask-cli deploys the lambda (along with the rest of the cloudformation infrastructure)
  4. ask-cli updates the skill manifest skill.json with the lambda endpoint and uploads the updated manifest to the SMAPI
  5. SMAPI responds with the validation error because the relative file path is not a valid URI unless as part of a skill package zip

I am currently blocked on this issue, so I'll see if I can fix the issue in the next few days. The three possible fixes I can think of:

  1. change the order (lambda + infrastructure first, then skill package) - cfn template requires Skill ID, so this won't work for first time
  2. use the manifest with the updated URLs when updating the manifest after setting up the lambda endpoint - I can't think of a clean way to do this without changing the user's manifest to point to the new URLs, which I don't think would be a great idea
  3. upload the updated manifest as part of a skill package zip when updating it. This is the one I think I'll move forward with.

I will be happy to submit my changes as a PR if you are interested.

@conman124
Copy link
Author

I've tried a few things to make it work and I can't quite get anything working well. I suppose I'm not really blocked because I can just upload my icons to an S3 bucket and point to them. However, I do think that ask-cli should support skill icons out of the box (no extra scripting required).

@kakhaUrigashvili
Copy link
Contributor

kakhaUrigashvili commented Jul 13, 2020

@conman124 seems like the root cause is that import skill package api and update manifest api have inconsistent validations for icon urls.

  1. we first upload skill package using the following steps
    https://github.com/alexa/ask-cli/blob/develop/lib/controllers/skill-metadata-controller/index.js#L198
    The validation for the urls passes.

  2. later when we get lambda arn we update the skill package:
    https://github.com/alexa/ask-cli/blob/develop/lib/controllers/skill-infrastructure-controller/index.js#L240
    The validation for the urls fails.

I notice this error happens only the first deploy when we don't have lambda url.

I think there is an easy fix.

instead of uploading skill.json from the file system, we can first call smapi get manifest api. Looks like after upload skill package urls are transformed to something like this:
"largeIconUri": "https://s3.amazonaws.com/CAPS-SSE/echo_developer/....."

you can see this by calling
'ask smapi get-skill-manifest -s skill id -g development'

So instead of calling

https://github.com/alexa/ask-cli/blob/develop/lib/controllers/skill-infrastructure-controller/index.js#L240

We call get skill manifest api, update the lambda url and, call update skill manifest api.

https://github.com/alexa/ask-cli/blob/develop/lib/controllers/skill-infrastructure-controller/index.js#L152

@RonWang do you have any concern about the solution?

@kakhaUrigashvili
Copy link
Contributor

@conman124 I have opened a PR #272

@RonWang
Copy link
Contributor

RonWang commented Aug 4, 2020

@conman124 We just released v2.14.0 which contains the fix of your issue. We replace the update-manifest call with the skillPackage update-package call to avoid the inter-operable issue between SMAPI and SkillPackage. The update will be longer but not much, as skillPackage update is smart when InteractionModel is not changing thus should be skipped. Close this issue accordingly.

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

No branches or pull requests

3 participants