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.

Sign up for GitHub

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

Jump to bottom

Minor improvements around logging and error reporting (in lib/lbt package) #29

Merged
merged 1 commit into from
Jun 25, 2018
Merged

Minor improvements around logging and error reporting (in lib/lbt package) #29

merged 1 commit into from
Jun 25, 2018

Conversation

codeworrior
Copy link
Member

  • all loggers in the lib/lbt modules now use the same hierarchical
    naming scheme as the other modules
  • component analyzers no longer fail but return silently when there's
    no manifest.json (often occurs in test data)
  • more log outputs of the JSModuleAnalyzer now have been classified as
    'verbose' to reduce the noise during build
  • usages of the Java method 'String.format' that had been missed
    during migration have been fixed

@coveralls
Copy link

coveralls commented Jun 22, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 180

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 62.864%
Totals Coverage Status
Change from base Build 167: 0.0%
Covered Lines: 425
Relevant Lines: 620
💛 - Coveralls

@codeworrior codeworrior requested review from matz3 and RandomByte June 22, 2018 08:40
matz3
matz3 requested changes Jun 25, 2018
View reviewed changes
Copy link
Member

@matz3 matz3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message should have a category (e.g. [INTERNAL])

@codeworrior
[INTERNAL] Minor improvements around logging and error reporting
e6e0db8 
- all loggers in the lib/lbt modules now use the same hierarchical
  naming scheme as the other modules
- component analyzers no longer fail but return silently when there's
  no manifest.json (often occurs in test data)
- more log outputs of the JSModuleAnalyzer now have been classified as
  'verbose' to reduce the noise during build
- usages of the Java method 'String.format' that had been missed
  during migration have been fixed
@codeworrior codeworrior force-pushed the improve-lbt-logging-and-error-reporting branch from b6bf115 to e6e0db8 Compare June 25, 2018 12:30
@stopcoder
Copy link
Member

component analyzers no longer fail but return silently when there's
no manifest.json (often occurs in test data)

👍👍👍👍👍

@codeworrior codeworrior merged commit 55b1e89 into master Jun 25, 2018
@stopcoder
Copy link
Member

Hi @codeworrior,

there's still error thrown from ResourcePool.js when some module can't be found:

ui5-builder/lib/lbt/resources/ResourcePool.js

Line 162 in e6e0db8

throw new Error("resource not found in pool: '" + name + "'");

Is there a plan to replace this with verbose as well?

Best regards,
Jiawei

@codeworrior
Copy link
Member Author

I considered this, but decided against it. In my opinion a configured but missing resource represents a significant configuration error.

Do you have a concrete scenario where it would make more sense to tolerate this?

@codeworrior codeworrior deleted the improve-lbt-logging-and-error-reporting branch June 25, 2018 14:53
@stopcoder
Copy link
Member

I agree with you. If a configured resource is missing, we definitely need to throw an error.

I was thinking under the context of the missing manifest file in component analyser and I pointed out the wrong place about the thrown error. If a component doesn't have a manifest, there's still error logged from the following line:

ui5-builder/lib/lbt/analyzer/ComponentAnalyzer.js

Line 62 in e6e0db8

log.error("an error occurred while analyzing component %s (ignored)", resource.name, err);

Does it make sense to replace this log.error with log.verbose? Because the missing manifest isn't configured anywhere and I find the following errors in the output too strong for a unintendedly missing manifest file.

ERR! ComponentAnalyzer Error: resource not found in pool: 'pony/dummy/defaults/manifest.json'
ERR! ComponentAnalyzer at LocatorResourcePool.findResource (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:162:10)
ERR! ComponentAnalyzer at ComponentAnalyzer.analyze (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/analyzer/ComponentAnalyzer.js:54:44)
ERR! ComponentAnalyzer at determineDependencyInfo (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:79:34)
ERR! ComponentAnalyzer at
ERR! ComponentAnalyzer at process._tickCallback (internal/process/next_tick.js:118:7)
ERR! ComponentAnalyzer an error occurred while analyzing component %s (ignored) pony/dummy/defaults/Component.js { Error: resource not found in pool: 'pony/dummy/defaults/manifest.json'
ERR! ComponentAnalyzer at LocatorResourcePool.findResource (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:162:10)
ERR! ComponentAnalyzer at ComponentAnalyzer.analyze (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/analyzer/ComponentAnalyzer.js:54:44)
ERR! ComponentAnalyzer at determineDependencyInfo (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:79:34)
ERR! ComponentAnalyzer at
ERR! ComponentAnalyzer at process._tickCallback (internal/process/next_tick.js:118:7)
ERR! ComponentAnalyzer stack: 'Error: resource not found in pool: 'pony/dummy/defaults/manifest.json'\n at LocatorResourcePool.findResource (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:162:10)\n at ComponentAnalyzer.analyze (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/analyzer/ComponentAnalyzer.js:54:44)\n at determineDependencyInfo (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:79:34)\n at \n at process._tickCallback (internal/process/next_tick.js:118:7)' }
ERR! SmartTemplateAnalyzer Error: resource not found in pool: 'pony/dummy/defaults/manifest.json'
ERR! SmartTemplateAnalyzer at LocatorResourcePool.findResource (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:162:10)
ERR! SmartTemplateAnalyzer at TemplateComponentAnalyzer.analyze (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/analyzer/SmartTemplateAnalyzer.js:57:44)
ERR! SmartTemplateAnalyzer at determineDependencyInfo (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:80:38)
ERR! SmartTemplateAnalyzer at
ERR! SmartTemplateAnalyzer at process._tickCallback (internal/process/next_tick.js:118:7)
ERR! SmartTemplateAnalyzer an error occurred while analyzing template app %s (ignored) pony/dummy/defaults/Component.js { Error: resource not found in pool: 'pony/dummy/defaults/manifest.json'
ERR! SmartTemplateAnalyzer at LocatorResourcePool.findResource (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:162:10)
ERR! SmartTemplateAnalyzer at TemplateComponentAnalyzer.analyze (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/analyzer/SmartTemplateAnalyzer.js:57:44)
ERR! SmartTemplateAnalyzer at determineDependencyInfo (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:80:38)
ERR! SmartTemplateAnalyzer at
ERR! SmartTemplateAnalyzer at process._tickCallback (internal/process/next_tick.js:118:7)
ERR! SmartTemplateAnalyzer stack: 'Error: resource not found in pool: 'pony/dummy/defaults/manifest.json'\n at LocatorResourcePool.findResource (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:162:10)\n at TemplateComponentAnalyzer.analyze (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/analyzer/SmartTemplateAnalyzer.js:57:44)\n at determineDependencyInfo (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:80:38)\n at \n at process._tickCallback (internal/process/next_tick.js:118:7)' }
ERR! FioriElementAnalyzer Error: resource not found in pool: 'pony/dummy/defaults/manifest.json'
ERR! FioriElementAnalyzer at LocatorResourcePool.findResource (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:162:10)
ERR! FioriElementAnalyzer at FioriElementsAnalyzer.analyze (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/analyzer/FioriElementsAnalyzer.js:98:44)
ERR! FioriElementAnalyzer at determineDependencyInfo (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:81:38)
ERR! FioriElementAnalyzer at
ERR! FioriElementAnalyzer at process._tickCallback (internal/process/next_tick.js:118:7)
ERR! FioriElementAnalyzer an error occurred while analyzing template app %s (ignored) pony/dummy/defaults/Component.js { Error: resource not found in pool: 'pony/dummy/defaults/manifest.json'
ERR! FioriElementAnalyzer at LocatorResourcePool.findResource (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:162:10)
ERR! FioriElementAnalyzer at FioriElementsAnalyzer.analyze (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/analyzer/FioriElementsAnalyzer.js:98:44)
ERR! FioriElementAnalyzer at determineDependencyInfo (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:81:38)
ERR! FioriElementAnalyzer at
ERR! FioriElementAnalyzer at process._tickCallback (internal/process/next_tick.js:118:7)
ERR! FioriElementAnalyzer stack: 'Error: resource not found in pool: 'pony/dummy/defaults/manifest.json'\n at LocatorResourcePool.findResource (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:162:10)\n at FioriElementsAnalyzer.analyze (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/analyzer/FioriElementsAnalyzer.js:98:44)\n at determineDependencyInfo (/workspace/ui5-cli/node_modules/@ui5/builder/lib/lbt/resources/ResourcePool.js:81:38)\n at \n at process._tickCallback (internal/process/next_tick.js:118:7)' }

@codeworrior
Copy link
Member Author

Strange.

These are exactly the messages that I suppressed with 55b1e89 (note the catch after each findResource call). And when I setup a similar scenario with a subcomponent but without manifest.json, I no longer get the errors.

The line numbers didn't change with my commit, so that part of the stack traces doesn't give a hint. But to me, the file names look suspicious (/workspace/ui5-cli/node_modules//lib/lbt/resources/ResourcePool.js). The lib folder is not a node module (package) of its own. Could you please double check whether the projects are correctly linked and whether the ui5-cli is using the expected version of the ui5-builder project?

If everything is fine, what node version are you using?

@stopcoder
Copy link
Member

Hi @codeworrior,

sorry that the npm link between ui5-cli and ui5-builder wasn't established correctly in my local workspace. I think I forgot to link to the ui5-builder in ui5-cli after creating the link in ui5-builder. 😓

I tested again and the ERRs are gone.

Best regards,
Jiawei

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matz3 matz3 matz3 approved these changes

@RandomByte RandomByte Awaiting requested review from RandomByte

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@codeworrior @coveralls @stopcoder @matz3