lando / platformsh

The Official Platform.sh Lando Plugin
https://docs.lando.dev/platformsh
GNU General Public License v3.0
6 stars 4 forks source link

Error with finding primary route on PlatformSH #110

Closed dustinleblanc closed 2 years ago

dustinleblanc commented 3 years ago

Getting this error:

Unhandled rejection TypeError: Cannot set property 'primary' of undefined
    at setPrimaryRoute (/snapshot/lando/build/cli/integrations/lando-platformsh/lib/config.js)
    at /snapshot/lando/build/cli/integrations/lando-platformsh/lib/config.js
    at Function.thru (/snapshot/lando/build/cli/node_modules/lodash/lodash.js:8817:14)
    at /snapshot/lando/build/cli/node_modules/lodash/lodash.js:4388:28
    at arrayReduce (/snapshot/lando/build/cli/node_modules/lodash/lodash.js:683:21)
    at baseWrapperValue (/snapshot/lando/build/cli/node_modules/lodash/lodash.js:4387:14)
    at LodashWrapper.wrapperValue (/snapshot/lando/build/cli/node_modules/lodash/lodash.js:9072:14)
    at Object.parseRoutes (/snapshot/lando/build/cli/integrations/lando-platformsh/lib/config.js)
    at AsyncEvents.<anonymous> (/snapshot/lando/build/cli/integrations/lando-platformsh/app.js)
    at AsyncEvents.handle (/snapshot/lando/build/cli/lib/events.js)
    at /snapshot/lando/build/cli/lib/events.js
From previous event:
    at AsyncEvents.emit (/snapshot/lando/build/cli/lib/events.js)
    at /snapshot/lando/build/cli/lib/app.js
From previous event:
    at App.init (/snapshot/lando/build/cli/lib/app.js)
    at /snapshot/lando/build/cli/bin/lando.js
From previous event:
    at Object.<anonymous> (/snapshot/lando/build/cli/bin/lando.js)
    at Module._compile (pkg/prelude/bootstrap.js:1324:22)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:993:10)
    at Module.load (internal/modules/cjs/loader.js:813:32)
    at Function.Module._load (internal/modules/cjs/loader.js:725:14)
    at Function.Module.runMain (pkg/prelude/bootstrap.js:1379:12)
    at internal/main/run_main_module.js:17:11

Here is the anonymized Lando config.

name: someclient
recipe: platformsh
excludes:
  - drush
  - vendor
  - web/core
  - web/modules/contrib
  - web/profiles/contrib
  - web/sites/default/files
  - web/themes/contrib
config:
  id: someid
proxy:
  mailhog:
    - mail-someclient.lndo.site
  node:
    - bs-someclient.lndo.site:3000
env_file:
  - .env
services:
  app:
    overrides:
      environment:
        DRUSH_OPTIONS_URI: http://someclient.lndo.site
  node:
    type: node
    command: cd /app/web/themes/custom/someclient_theme && yarn watch
    scanner: false
    ssl: true
    overrides:
      environment:
        BS_DOMAIN: "http://app"
        BS_SOCKET_DOMAIN: "https://bs-someclient.lndo.site"
        BS_SOCKET_PORT: 80
    build:
      - cd /app/web/themes/custom/someclient_theme && yarn install
      - cd /app/web/themes/custom/someclient_theme && yarn dev
  mailhog:
    type: mailhog
    hogfrom:
      - app
  d7db:
    type: mariadb:10.4
    config:
      database: config/my-custom.cnf
    creds:
      user: drupal7db
      password: drupal7db
      database: drupal7db
    portforward: true
tooling:
  drush:
    service: app
    cmd: /app/vendor/bin/drush --root=/app/web
  npm:
    service: node
  node:
    service: node
  yarn:
    service: node
    cmd: yarn

Looking at the setPrimaryRoute method, it assumes the incoming argument is an array, when I dump the value for this app, it is an empty object ({}).

dustinleblanc commented 3 years ago

Looking at this, it was because my .platform/routes.yaml was empty

pirog commented 3 years ago

@dustinleblanc does platform REQUIRE a route exist? Seems like no?

dustinleblanc commented 3 years ago

@pirog from what I can tell, our code requires a route to exist, not sure if the platform, in general, requires it. If I tell this function to just skip manipulating the routes if they are empty, I get an error in a different part of the code. I was actually thinking of something like this:

/*
 * Helper to set primary route if needed
 */
const setPrimaryRoute = (routes = {}) => {
  // If we dont have a primary then set one
  if (_.isEmpty(routes)) {
    routes = {
      'http://{default}/': {
        type: 'upstream',
        primary: true,
        upstream: "app:http"
      }
    }
  }
  if (!_.some(routes, 'primary')) {
    const firstUpstream = _.find(routes, {type: 'upstream'});
    firstUpstream.primary = true;
  }

  // Return
  return routes;
};

That feels like a safe assumption, that if someone set up an app but hadn't yet configured routes for the platform, that we'd just have a default route that loads the app container, but I haven't PR'd that because I wasn't sure if that was a good idea or not.

We could set aside some time to try testing an app spin up with no routes defined, but I don't have enough knowledge of their setup yet to know for sure what happens.

pirog commented 3 years ago

@dustinleblanc ok, let me take some time to think about how we should handle this. To replicate i assume i just delete my routes file and lando rebuild ?

dustinleblanc commented 3 years ago

Yeah that seems to be what caused it. I was trying to get rid of any extra cruft in that directory, but I didn't realize it had important files in it.

pirog commented 3 years ago

I can replicate!