MaxSem has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/316024

Change subject: Improve error handling
......................................................................

Improve error handling

Change-Id: I52b73e228e6cb232390bcbbb00d7c7aca161c063
---
M i18n/en.json
M i18n/qqq.json
M includes/ParserTag.php
M tests/parserTests.txt
4 files changed, 16 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Graph 
refs/changes/24/316024/1

diff --git a/i18n/en.json b/i18n/en.json
index 56c2f6d..10f53fa 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -3,7 +3,8 @@
                "authors": [
                        "Dan Andreescu",
                        "Yuri Astrakhan",
-                       "Frédéric Bolduc"
+                       "Frédéric Bolduc",
+                       "Max Semenik"
                ]
        },
        "graph-desc": "Allows <graph> tags or entire pages to become 
[http://vega.github.io/ Vega]-based graphs",
@@ -14,6 +15,8 @@
        "graph-broken-category-desc": "The page includes a 
<code><nowiki><graph></nowiki></code> tag with invalid usage.",
        "graph-obsolete-category": "Pages with obsolete Vega 1.0 graphs",
        "graph-obsolete-category-desc": "The page includes a 
<code><nowiki><graph></nowiki></code> tag that should be updated to version 2.",
+       "graph-error-empty-json": "Empty graph data, nothing to show",
+       "graph-error-not-vega": "The JSON provided is not a valid Vega JSON",
        "apihelp-graph-description": "Access graph tag functionality.",
        "apihelp-graph-example": "Get the graph JSON by hash and title",
        "apihelp-graph-param-hash": "Hash value of the graph",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index a509191..66b6606 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -16,6 +16,8 @@
        "graph-broken-category-desc": "Description on 
[[Special:TrackingCategories]] for the {{msg-mw|graph-broken-category}} 
tracking category.",
        "graph-obsolete-category": "Name of the tracking category",
        "graph-obsolete-category-desc": "Description on 
[[Special:TrackingCategories]] for the {{msg-mw|graph-obsolete-category}} 
tracking category.",
+       "graph-error-empty-json": "Shown by &lt;graph&gt; when used without any 
data in it, e.g. &lt;graph&gt;&lt;/graph&gt;",
+       "graph-error-not-vega": "Shown by &lt;graph&gt; when used with invalid 
data, e.g. &lt;graph&gt;[]&lt;/graph&gt;",
        "apihelp-graph-description": "{{doc-apihelp-description|graph}}",
        "apihelp-graph-example": "{{doc-apihelp-example|graph}}",
        "apihelp-graph-param-hash": "{{doc-apihelp-param|graph|hash}}",
diff --git a/includes/ParserTag.php b/includes/ParserTag.php
index a4fe751..a681a08 100644
--- a/includes/ParserTag.php
+++ b/includes/ParserTag.php
@@ -142,6 +142,10 @@
        public function buildHtml( $jsonText, Title $title, $revid, $args = 
null ) {
                global $wgGraphImgServiceUrl, $wgServerName;
 
+               $jsonText = trim( $jsonText );
+               if ( $jsonText === '' ) {
+                       return $this->formatError( wfMessage( 
'graph-error-empty-json' ) );
+               }
                $status = FormatJson::parse( $jsonText, FormatJson::TRY_FIXING 
| FormatJson::STRIP_COMMENTS );
                if ( !$status->isOK() ) {
                        return $this->formatStatus( $status );
@@ -151,8 +155,7 @@
                $graphTitle = isset( $args['title'] ) ? $args['title'] : '';
                $data = $status->getValue();
                if ( !is_object( $data ) ) {
-                       // @todo: Output an error message instead?
-                       $data = (object)[ 'width' => 200, 'height' => 200 ];
+                       return $this->formatError( wfMessage( 
'graph-error-not-vega' ) );
                }
 
                // Figure out which vega version to use
diff --git a/tests/parserTests.txt b/tests/parserTests.txt
index ba0983b..435302d 100644
--- a/tests/parserTests.txt
+++ b/tests/parserTests.txt
@@ -28,10 +28,10 @@
 [ "foo", "bar" ]
 </graph>
 !! result
-<div class="mw-graph mw-graph-always" style="min-width:200px;min-height:200px" 
data-graph-id="4406ae21fb62ba4a67e041d0f9e026a2819e48a8"></div>
-<div class="mw-graph mw-graph-always" style="min-width:200px;min-height:200px" 
data-graph-id="4406ae21fb62ba4a67e041d0f9e026a2819e48a8"></div>
-<p><span class="error">Syntax error</span>
+<p><span class="error">Empty graph data, nothing to show</span>
+<span class="error">Empty graph data, nothing to show</span>
+<span class="error">Syntax error</span>
+<span class="error">The JSON provided is not a valid Vega JSON</span>
 </p>
-<div class="mw-graph mw-graph-always" style="min-width:200px;min-height:200px" 
data-graph-id="4406ae21fb62ba4a67e041d0f9e026a2819e48a8"></div>
 
-!! end
+ !! end

-- 
To view, visit https://gerrit.wikimedia.org/r/316024
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I52b73e228e6cb232390bcbbb00d7c7aca161c063
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Graph
Gerrit-Branch: master
Gerrit-Owner: MaxSem <maxsem.w...@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to