Navigation Menu

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

Better diagnostics with CLI command lb4 repository #3965

Closed
ibmjm opened this issue Oct 17, 2019 · 5 comments · Fixed by #4048
Closed

Better diagnostics with CLI command lb4 repository #3965

ibmjm opened this issue Oct 17, 2019 · 5 comments · Fixed by #4048
Labels
developer-experience Issues affecting ease of use and overall experience of LB users help wanted

Comments

@ibmjm
Copy link
Contributor

ibmjm commented Oct 17, 2019

Steps to reproduce

Edit one of your datasource JSON files to contain a trailing comma on the last key/value pair (or any other violation of JSON standards).
Run lb4 repository and observe the uncaught error. Notice that the error explains that a JSON parsing error occurred, but says it happened in anonymous JSON, instead of naming the problematic file.

Current Behavior

An error is thrown, indicating that JSON parsing failed, but not which datasource file is to blame:

      throw er; // Unhandled 'error' event
      ^

SyntaxError: Unexpected token } in JSON at position 198
    at JSON.parse (<anonymous>)
    at Object.exports.isConnectorOfType (/usr/local/lib/node_modules/@loopback/cli/lib/utils.js:510:28)
    at datasourcesList.filter.item (/usr/local/lib/node_modules/@loopback/cli/generators/repository/index.js:258:28)
    at Array.filter (<anonymous>)
    at RepositoryGenerator.promptDataSourceName (/usr/local/lib/node_modules/@loopback/cli/generators/repository/index.js:256:50)
Emitted 'error' event at:
    at Immediate.setImmediate (/usr/local/lib/node_modules/@loopback/cli/node_modules/yeoman-generator/lib/index.js:404:18)
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)

Expected Behavior

Desired outcome would be that the name of the file containing the illegally-formatted JSON would be listed, so the developer would quickly be able to fix the file.

Something more like:

SyntaxError: Unexpected token } in JSON at  position 198
    at JSON.parse ("src/datasources/{name-of-your-datasource}.datasource.json")

carry on with stack trace here

Link to reproduction sandbox

I tried to follow https://loopback.io/doc/en/contrib/Reporting-issues.html#loopback-4x-bugs instructions by forking + branching, then committing a change to one of the datasources in the To Do example. The change consists only of adding a trailing comma to the last entry in the src/db.datasource.json file so that it violates JSON formatting rules.

Additional information

jons-mbp:~/iar/api $ node -e 'console.log(process.platform, process.arch, process.versions.node)'
darwin x64 10.15.1
jons-mbp:~/iar/api $ npm ls --prod --depth 0 | grep loopback
├── @loopback/authentication@3.0.1
├── @loopback/boot@1.5.6
├── @loopback/context@1.23.0
├── @loopback/core@1.10.2
├── @loopback/openapi-v3@1.9.7
├── @loopback/repository@1.14.0
├── @loopback/rest@1.19.0
├── @loopback/rest-explorer@1.3.7
├── @loopback/service-proxy@1.3.6
├── loopback-connector-db2@2.2.0
├── loopback-connector-mysql@5.4.2
├── loopback-connector-rest@3.5.0
jons-mbp:~/iar/api $

Related Issues

@ibmjm ibmjm added the bug label Oct 17, 2019
@dhmlau
Copy link
Member

dhmlau commented Oct 17, 2019

Thanks @ibmjm for opening the issue.

Capturing the discussion with @bajtos:

Here is the problematic code: https://github.com/strongloop/loopback-next/blob/eef77d58ff359eb049213982a8d2205a1d3c9151/packages/cli/lib/utils.js#L508-L514

As a workaround:

You can also run lb4 with debug enabled, I think that will show you which file is malformed.
$ DEBUG=loopback:cli:utils lb4 repository

@dhmlau dhmlau added developer-experience Issues affecting ease of use and overall experience of LB users help wanted and removed bug labels Oct 17, 2019
@ibmjm
Copy link
Contributor Author

ibmjm commented Oct 17, 2019

You can also run lb4 with debug enabled, I think that will show you which file is malformed.
$ DEBUG=loopback:cli:utils lb4 repository

Confirmed, that was how I traced which datasource file was malformed.

I think it would be more helpful if the error message included the filename all the time. (I acknowledge that I don't know what it would report in a fully built distribution running on production, so maybe that's not possible.) Either way, it seems that the error being thrown from @loopback/core/packages/cli/lib/utils.js that's attempting the JSON parsing, isn't being caught by the lb4 repository command...

@bajtos
Copy link
Member

bajtos commented Nov 1, 2019

+1 to make the path to the malformed JSON file more visible.

Would you @ibmjm like to contribute such improvement yourself?

See our Contributing guide and Submitting a pull request to LoopBack 4 to get started.

@ibmjm
Copy link
Contributor Author

ibmjm commented Nov 1, 2019

+1 to make the path to the malformed JSON file more visible.

Would you @ibmjm like to contribute such improvement yourself?

See our Contributing guide and Submitting a pull request to LoopBack 4 to get started.

I am actually not 100% sure what the best approach would be. My naive suggestion looks like this:

jons-mbp:~/loopback/lb4-bug-json $ git diff packages
diff --git a/packages/cli/lib/utils.js b/packages/cli/lib/utils.js
index f60deb69..e89f26b2 100644
--- a/packages/cli/lib/utils.js
+++ b/packages/cli/lib/utils.js
@@ -426,7 +426,8 @@ exports.getDataSourceConnectorName = function(datasourcesDir, dataSourceClass) {
     jsonFileContent = JSON.parse(fs.readFileSync(datasourceJSONFile, 'utf8'));
   } catch (err) {
     debug(`Error reading file ${datasourceJSONFile}: ${err.message}`);
-    throw err;
+    throw new Error(`Error reading file ${datasourceJSONFile}: ${err.message}`);
   }
 
   if (jsonFileContent.connector) {
jons-mbp:~/loopback/lb4-bug-json $ 

If something that simple seems feasible to you, I can try to build a pull request next week when my time frees up.

@bajtos
Copy link
Member

bajtos commented Nov 5, 2019

@ibmjm your solution is reasonable, but let's preserve the error stack please.

jons-mbp:~/loopback/lb4-bug-json $ git diff packages
diff --git a/packages/cli/lib/utils.js b/packages/cli/lib/utils.js
index f60deb69..e89f26b2 100644
--- a/packages/cli/lib/utils.js
+++ b/packages/cli/lib/utils.js
@@ -426,7 +426,8 @@ exports.getDataSourceConnectorName = function(datasourcesDir, dataSourceClass) {
     jsonFileContent = JSON.parse(fs.readFileSync(datasourceJSONFile, 'utf8'));
   } catch (err) {
     debug(`Error reading file ${datasourceJSONFile}: ${err.message}`);
+    err.message = `Cannot load ${datasourceJSONFile}: ${err.message}`;
     throw err;
   }
 
   if (jsonFileContent.connector) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Issues affecting ease of use and overall experience of LB users help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants