-
-
Notifications
You must be signed in to change notification settings - Fork 146
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
Log all error and warning in the console, and display them #5424
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to not import mainLizmap
in modules
@@ -498,8 +499,7 @@ export default class Action { | |||
this.ACTIVE_LIZMAP_ACTION = this.buildActionInstanceUniqueId(action.name, scope, layerId, featureId); | |||
|
|||
} catch (error) { | |||
// Display the error | |||
console.warn(error); | |||
mainLizmap._lizmap3.addMessage(lizDict['action.error'], 'danger', true, error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mainLizmap._lizmap3.addMessage(lizDict['action.error'], 'danger', true, error); | |
this._lizmap3.addMessage(lizDict['action.error'], 'danger', true, error); |
mainLizmap
is not needed, the old lizmap3 is already available
@@ -12,6 +12,7 @@ import GeoJSON from 'ol/format/GeoJSON.js'; | |||
import Point from 'ol/geom/Point.js'; | |||
import { fromExtent } from 'ol/geom/Polygon.js'; | |||
import WKT from 'ol/format/WKT.js'; | |||
import {mainLizmap} from "./Globals.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import {mainLizmap} from "./Globals.js"; |
mainLizmap
is not needed, the old lizmap3 is already available
I was quickly thinking : stateDiagram-v2
file_a : File A.js
file_b : File B.js
log : Logger (according to level of message, current login, state...)
console: Console
ui: UI generic error message
ui_admin: UI with connected as admin, more complete error message
server: Log on the server, a new jLog client-side-errors.log
file_a --> log
file_b --> log
log --> ui
log --> ui_admin
log --> server
log --> console
Most of logger in desktop applications have this kind of setup. |
Linked to #4938
Similar request in my PR #5304 : An error was raised in the console about invalid layer, but the admin user was not aware about it.
I think all
console.warn
andconsole.error
should be reviewed, and we should use a kind of proxy to display in UI the error message, and a more detailed error message if the user is an admin.Errors logged in the console are not visible, which make difficult for Lizmap users who doesn't know this feature and do not think about it.
I think only the "proxy" should do the
console.error
, and a corresponding Javascript error must be displayed with a collapsible div like :An error occured while running the action "name"
See more detail about the error
Error: No fill, stroke, point, or text symbolizer properties in style: {"graphicName":"circle","pointRadius":6,"fill":true,"fillColor":"lightred","fillOpacity":0.3,"stroke":true,"strokeWidth":4,"strokeColor":"red","strokeOpacity":0.8}
PR totally broken for now, but I want some ideas before digging. I noticed there are different
addMessage
, there is also aMessage
component.