Logging for XUL template based on storage

RESOLVED FIXED in mozilla1.9.3a3

Status

()

--
enhancement
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: laurent, Assigned: laurent)

Tracking

Trunk
mozilla1.9.3a3
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
We have now logging capability into the xul template engine (bug 321169). The storage query processor should use this logging system, to inform about errors which could appear (no database, bad sql query etc..).

I'm working on a patch.
(Assignee)

Updated

9 years ago
Status: NEW → ASSIGNED
Depends on: 321169
(Assignee)

Updated

9 years ago
Assignee: nobody → laurent
(Assignee)

Comment 1

9 years ago
Created attachment 425191 [details] [diff] [review]
The patch
Attachment #425191 - Flags: superreview?(neil)
Attachment #425191 - Flags: review?(enndeakin)
Comment on attachment 425191 [details] [diff] [review]
The patch

>+#define ERROR_TEMPLATE_STORAGE_UNKNOW_QUERY_PARAMETER                   \
Typo: _UNKNOWN_

>+            if (typeError < 1) {
Nit: I'd prefer <= 0
Attachment #425191 - Flags: superreview?(neil) → superreview+

Comment 3

9 years ago
Comment on attachment 425191 [details] [diff] [review]
The patch

>+#define ERROR_TEMPLATE_STORAGE_BAD_URI                                  \
>+        "Only profile: or file URI are allowed"
>+#define ERROR_TEMPLATE_STORAGE_CANNOT_OPEN_DATABASE                     \
>+        "Cannot open given database"
>+#define ERROR_TEMPLATE_STORAGE_BAD_QUERY                                \
>+        "Syntax error in the SQL query"
>+#define ERROR_TEMPLATE_STORAGE_UNKNOW_QUERY_PARAMETER                   \
>+        "The given named parameter is unknown in the SQL query"
>+#define ERROR_TEMPLATE_STORAGE_WRONG_TYPE_QUERY_PARAMETER               \
>+        "The type of a query parameter is wrong"
>+#define ERROR_TEMPLATE_STORAGE_QUERY_PARAMETER_NOT_BOUND                \
>+        "A query parameter cannot be bound to the SQL query"

The existing errors begin with a lowercase letter.


>+expectedConsoleMessages.push("Error parsing template: The given named parameter is unknown in the SQL query");
>+expectedConsoleMessages.push("Error parsing template: The type of a query parameter is wrong");
>+expectedConsoleMessages.push("Error parsing template: A query parameter cannot be bound to the SQL query");

Although it's probably ok, this relies on the templates being parsed in order, which isn't necessarily true. You may want to at least add a note here about this.
Attachment #425191 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 4

9 years ago
Created attachment 430718 [details] [diff] [review]
The patch to commit
[Checkin: Comment 6]

I split the test into three files, so we won't have a potential problem with the order of messages.
Attachment #425191 - Attachment is obsolete: true
(Assignee)

Comment 5

9 years ago
tinderboxes status is currently orange...
Keywords: checkin-needed
Comment on attachment 430718 [details] [diff] [review]
The patch to commit
[Checkin: Comment 6]


http://hg.mozilla.org/mozilla-central/rev/1cb03235231e
Attachment #430718 - Attachment description: The patch to commit → The patch to commit [Checkin: Comment 6]
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a3
You need to log in before you can comment on or make changes to this bug.