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

Parsing trouble for non-required extension parameters #3934

Closed
alallema opened this issue Dec 7, 2021 · 1 comment · Fixed by #3951
Closed

Parsing trouble for non-required extension parameters #3934

alallema opened this issue Dec 7, 2021 · 1 comment · Fixed by #3951
Assignees

Comments

@alallema
Copy link

alallema commented Dec 7, 2021

[REQUIRED] Environment info

firebase-tools: 9.23.0

Platform: macOS

[REQUIRED] Test case

I'm not sure if it's a regression, but I found this issue that seems pretty similar.
Installs an extension installation or emulator with the --param file as an argument. If there are optional parameters that are empty in the file, the next line will be read.
This issue seems to be present in the v9.22.0 but not in the v9.21.0

[REQUIRED] Steps to reproduce

This is the extension.yml file:

  - param: FIELDS_TO_INDEX
    label: Fields to index in Meilisearch
    description: >-
        What are the names of the fields you want to be indexed in MeiliSearch?
        Create a comma-separated list of the fields name, or leave blank to take all fields.
        The id field is always indexed even when omitted in the list.
    example: 'title,overview,release_date'
    validationRegex: "^[^,]?[a-zA-Z-_0-9,]*[^,]$"
    validationErrorMessage:
      Fields must be given through a comma-separated list.
    default: ''
    required: false

  - param: SEARCHABLE_FIELDS
    label: Fields in which to search. - Optional
    description: >-
      What are the names of the fields you want to make searchable in MeiliSearch?
      This features is optional. See [the documentation about it](https://docs.meilisearch.com/reference/features/field_properties.html#searchable-fields)
      for more information.
      Create a comma-separated list of fields, or leave blank to include all fields.
    example: 'title,overview,release_date'
    validationRegex: "^[^,]?[a-zA-Z-_0-9,]*[^,]$"
    validationErrorMessage:
      Fields must be given through a comma-separated list.
    default: ''
    required: false

This is the param.env file:

LOCATION=us-central1
COLLECTION_PATH=movies
FIELDS_TO_INDEX=
SEARCHABLE_FIELDS=
MEILISEARCH_INDEX_NAME=movies
MEILISEARCH_HOST='http://127.0.0.1:7700'
MEILISEARCH_API_KEY='masterKey'
firebase ext:install <extension-name> --params=params.env --project=<project-id>

[REQUIRED] Expected behavior

The optional parameter does not have to be filled in.

[REQUIRED] Actual behavior

This is a print of process.env

{"location":"us-central1","collectionPath":"movies","host":"http://127.0.0.1:7700","apiKey":"masterKey","indexUid":"movies","fieldsToIndex":"SEARCHABLE_FIELDS=","searchableFields":"","severity":"INFO","message":"Initializing extension with configuration"}

Output:

Error: ; RESOURCE_ERROR at /deployments/firebase-ext-firestore-meilisearch-search-9djc/resources/indexingWorker: {"ResourceType":"gcp-types/cloudfunctions-v1:projects.locations.functions","ResourceErrorCode":"400","ResourceErrorMessage":"Function failed on loading user code. This is likely due to a bug in the user code. Error message: Error: please examine your function logs to see the error cause: https://cloud.google.com/functions/docs/monitoring/logging#viewing_logs. Additional troubleshooting documentation can be found at https://cloud.google.com/functions/docs/troubleshooting#logging. Please visit https://cloud.google.com/functions/docs/troubleshooting for in-depth troubleshooting documentation."}

Output with the param file modified as follows:

LOCATION=us-central1
COLLECTION_PATH=movies
FIELDS_TO_INDEX=''
SEARCHABLE_FIELDS=
MEILISEARCH_INDEX_NAME=movies
MEILISEARCH_HOST='http://127.0.0.1:7700'
MEILISEARCH_API_KEY='masterKey'
Error: MEILISEARCH_INDEX_NAME has not been set in the given params file and there is no default available. Please set this variable before installing again.
@joehan joehan self-assigned this Dec 13, 2021
@joehan
Copy link
Contributor

joehan commented Dec 13, 2021

Thanks for reporting this @alallema . Looks like I introduced this bug - between 9.21.0 & 9.22.0, I switched this code over from using https://www.npmjs.com/package/dotenv to use our own implementation of a dotenv parser. Our implementation has the benefit of supporting multiline variables, but it seems to be misbehaving for empty lines - I'll try to figure out why and get a fix out ASAP.

joehan added a commit that referenced this issue Dec 15, 2021
…ine values (#3934) (#3951)

* Fixing a bug where empty variables in .env files were read as multi-line values

* changelog

* take 2

* formats

* adding another test case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants