[strong-error-handler] Documentation impact

[crandmck]

Added https://docs.strongloop.com/display/APIC/Using+strong-error-handler that incorporates the README content.

From @loay:

• The express error-handler was basically meant for development modes and we had to add few properties to disable stack traces in our Loopback modules in order to prevent leaking sensitive information. Strong-error-handler works for both environments as written in the description and can protect the leak of sensitive info in production.
• It is already in Loopback scafolldiing and gets generated with new loopback apps.
• I fail to see the relation with the debug strings, which is meant for debugging and not error handling.
  • Yes, but strong-error-handler has a "debug mode", and first sentence of README says "...for use in development (debug)..." so it begs the question. I think the key distinction that I was reaching for is that strong-error-handler only affects HTTP response content and return codes, NOT anything written to the console (by default). In contrast the LB debug strings only affect info printed to the console. ... That may be obvious to you (us), but not necc. to user immediately. Please confirm.

From @bajtos:
I think there should be a brief description of why someone should use this module.
See the information in strongloop/loopback#1650
It would be great to copy useful bits from that issue into the official LoopBack docs, possibly to strong-error-handler's README too.

[crandmck]

@loay Please confirm that this is added in loopback v. 2.31.0.

Also confirm that I should note in config.json / remoting properties that disabling stack traces with

"errorHandler": {
      "disableStackTrace": false
    }

is only recommended with pre-2.31.0 versions.

[bajtos]

Also confirm that I should note in config.json / remoting properties that disabling stack traces with

"errorHandler": {
      "disableStackTrace": false
    }

is only recommended with pre-2.31.0 versions.

@crandmck it is recommended for all 2.x versions. disableStackTrace was removed in 3.x only. See https://github.com/strongloop/strong-remoting/blob/0cfb6cbc634c46664f806bdd219b8788a2ec18f1/3.0-RELEASE-NOTES.md#new-error-handler-for-rest-adapter

Now I see that release notes don't include instructions for upgrading from 2.x to 3.x, we need to fix that (I guess either me or @loay).

[ebarault]

in Migrating to 3.0 note, it is stated that one shall set in config.json

{
  ...
  "remoting": {
    "errorHandler": {
      "debug": true,
      "log": false
    }
  }
}

and in middleware.json

{
  "final:after": {
    "strong-error-handler": {
      "params": {
         "debug": false,
         "log": true,
       }
    }
  }
}

but i can't find any impact of the latest on the overall app behavior. Removing the config lines in middleware.json does work the exact same way. I'm confused about the requirement to have this config in middleware.json.

Could someone from the lb team clarify this?
(maybe something to do with strongloop/loopback#1762?)

---

also if i programmatically specify the config.json file to use, using NODE_ENV var, i seem to have a conflict when using the config file name config.development.json : the params from this file overrides the params from config.json even when not setting NODE_ENV=development. If i change for config.dev.json it's ok.
Is this a reserved file name?

[crandmck]

@loay @bajtos ^ ... ?

[bajtos]

{
  ...
  "remoting": {
    "errorHandler": {
      "debug": true,
      "log": false
    }
  }
}

Ah, that part is not correct. We should disable the built-in error handler completely, as can be seen in the template used for new projects (source):

{
  ...
  "remoting": {
    ...
    handleErrors: false
  }
}

also if i programmatically specify the config.json file to use, using NODE_ENV var, i seem to have a conflict when using the config file name config.development.json : the params from this file overrides the params from config.json even when not setting NODE_ENV=development. If i change for config.dev.json it's ok.
Is this a reserved file name?

LoopBack uses development as the default value when NODE_ENV is not provided.

[ebarault]

hey @bajtos
i had already tried to set handleErrors: false in config.json
the thing is, whatever i put in config.json, the params i set in middleware.json have absolutely no effect.

{
  "final:after": {
    "strong-error-handler": {
      "params": {
         "debug": true,
         "log": false,
       }
    }
  }
}

does not disable logs and does not enable detailed debug info over rest

---

as for setting NODE_ENV=development : ok, makes sense. but it's not sufficiently clear in the doc i guess

[bajtos]

the thing is, whatever i put in config.json, the params i set in middleware.json have absolutely no effect.

Well, I am afraid I have run out of obvious advices I can give. Could you please create a small app reproducing the issue (see http://loopback.io/doc/en/contrib/Reporting-issues.html#bug-report) and open a new issue in https://github.com/strongloop/loopback/issues?

[bajtos]

as for setting NODE_ENV=development : ok, makes sense. but it's not sufficiently clear in the doc i guess

@ebarault I see. Can you advise us how to make this fact more clear? Or perhaps even better, send a pull request with the proposed changes to https://github.com/strongloop/loopback.io?

[ebarault]

Well, I am afraid I have run out of obvious advices I can give

--> 😜
ok, will provide a demo app

about doc : ok, will do

[ebarault]

@bajtos :

LoopBack uses development as the default value when NODE_ENV is not provided.

am i right to think that this statement should apply application wide?

at the moment with latest loopback v3, the file datasources.json seems mandatory, even if my app has a datasources.development.json file and is run with no NODE_ENV set or NODE_ENV set to developement

WARNING: Main config file "datasources.json" is missing

see this very basic sandbox if you will : https://github.com/ebarault/loopback-sandbox/tree/testing/strong-error-handler-config

---

update: if a datasources.json file containing just {} is present, this will remove the warning and the app will take the information from datasources.development.json, i guess it is not expected and the app should work just with a single datasources.development.json file

[bajtos]

at the moment with latest loopback v3, the file datasources.json seems mandatory, even if my app has a datasources.development.json file and is run with no NODE_ENV set or NODE_ENV set to developement

Yes, datasources.json are mandatory. In fact, all configuration files (config, model-config, datasources, etc.) require the main .json file to be present, even if there are environment-specific overrides. Loosely related discussions: strongloop/loopback-boot#80 and strongloop/loopback-boot#71.

if a datasources.json file containing just {} is present, this will remove the warning and the app will take the information from datasources.development.json

IIRC, the code merging datasources.json anddatasources.development.json will ignore any datasources that are defined in .development.json but not defined in the master .json file. In other words, if your datasources.json is empty, then the app should not have any datasources configured.

[bajtos]

LoopBack uses development as the default value when NODE_ENV is not provided.

am i right to think that this statement should apply application wide?

To be more precise, this applies to all things loaded via loopback-boot@2, see https://github.com/strongloop/loopback-boot/blob/9cb3d0648b7d39e17e1e3e47ff26a098c44a3754/lib/compiler.js#L40.

[ebarault]

thanks for digging into it @bajtos. Clarifications are much appreciated.
Just to be exhaustive: regarding

the code merging datasources.json and datasources.development.json will ignore any datasources that are defined in .development.json but not defined in the master .json file.

this is not the case: if i have just {} in datasources.json, all datasources in datasources.development.json are set up

[bajtos]

the code merging datasources.json and datasources.development.json will ignore any datasources that are defined in .development.json but not defined in the master .json file.

this is not the case: if i have just {} in datasources.json, all datasources in datasources.development.json are set up

@ebarault You are right, the behaviour was changed in July 2015 by strongloop/loopback-boot#143. I was not invited to review that patch, which explains why I was not aware of this change.

[crandmck]

Is there anything further we need to add/change in the docs? Or can we close this issue?

[bajtos]

I think this is ok to be closed.

@ebarault if you think our documentation still needs changes, then please open a new issue.