Return result in standard GraphQL format

Success case: {data = ...}
Error case: {errors = {message = ..., ...}}

compiled_query:execute(...) becomes exception-safe (performs pcall
internally), but graphql.new(...):compile() and
graphql.new(...).execute(...) still can throw an exception.

Enabled 5_2 test in common.test.lua, it fails before fix for #135 (
PR #178).

Increased timeout to 10 seconds in pcre.test.lua and
nested_args.test.lua (#137).

Prerequisite for #71.
Prerequisite for #134.
Fixes #137.

Author: Totktonada

graphql/impl.lua

@@ -41,8 +41,15 @@ local function gql_execute(qstate, variables, operation_name)
 
     local root_value = {}
 
-    return execute(state.schema, qstate.ast, root_value, variables,
-        operation_name)
+    local ok, data = pcall(execute, state.schema, qstate.ast, root_value,
+        variables, operation_name)
+    if not ok then
+        local err = utils.serialize_error(data)
+        return {errors = {err}}
+    end
+    return {
+        data = data,
+    }
 end
 
 --- Compile a query and execute an operation.

graphql/server/server.lua

@@ -114,16 +114,8 @@ function server.init(graphql, host, port)
             operation_name = nil
         end
 
-        local ok, result = pcall(compiled_query.execute, compiled_query,
-            variables, operation_name)
-        if not ok then
-            return {
-                status = 200,
-                body = json.encode({error = {{message = result}}})
-            }
-        end
+        local result = compiled_query:execute(variables, operation_name)
 
-        result = {data = result}
         return {
             status = 200,
             headers = {

graphql/utils.lua

@@ -1,6 +1,8 @@
 --- Various utility function used across the graphql module sources and tests.
 
+local json = require('json')
 local log = require('log')
+local ffi = require('ffi')
 
 local utils = {}
 
@@ -217,4 +219,25 @@ function utils.optional_require_rex()
     return rex, is_pcre2
 end
 
+function utils.serialize_error(err)
+    if type(err) == 'string' then
+        return {message = err}
+    elseif type(err) == 'cdata' and
+            tostring(ffi.typeof(err)) == 'ctype<const struct error &>' then
+        return {message = tostring(err)}
+    elseif type(err) == 'table' and type(err.message) == 'string' then
+        return err
+    end
+
+    local message = 'internal error: unknown error format'
+    local encode_use_tostring_orig = json.cfg.encode_use_tostring
+    json.cfg({encode_use_tostring = true})
+    local orig_error = json.encode(err)
+    json.cfg({encode_use_tostring = encode_use_tostring_orig})
+    return {
+        message = message,
+        orig_error = orig_error,
+    }
+end
+
 return utils

test/bench/bench.lua

@@ -49,13 +49,13 @@ local function workload(shard, bench_prepare, bench_iter, opts)
 
     -- first iteration; print result and update checksum
     local result = bench_iter(state)
-    local result_str = yaml.encode(result)
+    local result_str = yaml.encode(result.data)
     checksum:update(result_str .. '1')
 
     -- the rest iterations; just update checksum
     for i = 2, iterations do
         local result = bench_iter(state)
-        local result_str = yaml.encode(result)
+        local result_str = yaml.encode(result.data)
         checksum:update(result_str .. tostring(i))
         if i % 100 == 0 then
             fiber.yield()

test/common/directives.test.lua

@@ -59,7 +59,7 @@ local function run_queries(gql_wrapper)
             first_name: Ivan
     ]]):strip())
 
-    test:is_deeply(result_1_1, exp_result_1_1, '1_1')
+    test:is_deeply(result_1_1.data, exp_result_1_1, '1_1')
 
     -- }}}
     -- {{{ 1_2
@@ -81,7 +81,7 @@ local function run_queries(gql_wrapper)
           description: first order of Ivan
     ]]):strip())
 
-    test:is_deeply(result_1_2, exp_result_1_2, '1_2')
+    test:is_deeply(result_1_2.data, exp_result_1_2, '1_2')
 
     -- }}}
 
@@ -122,7 +122,7 @@ local function run_queries(gql_wrapper)
           description: first order of Ivan
     ]]):strip())
 
-    test:is_deeply(result_2_1, exp_result_2_1, '2_1')
+    test:is_deeply(result_2_1.data, exp_result_2_1, '2_1')
 
     -- }}}
     -- {{{ 2_2
@@ -148,7 +148,7 @@ local function run_queries(gql_wrapper)
             first_name: Ivan
     ]]):strip())
 
-    test:is_deeply(result_2_2, exp_result_2_2, '2_2')
+    test:is_deeply(result_2_2.data, exp_result_2_2, '2_2')
 
     -- }}}
 

test/common/limit_result.test.lua

@@ -33,20 +33,25 @@ local function run_queries(gql_wrapper)
     local variables = {
         user_id = 5,
     }
-    local ok, result = pcall(gql_query.execute, gql_query, variables)
-    assert(ok == false, "this test should fail")
-    test:like(result,
-              'count%[4%] exceeds limit%[3%] %(`resulting_object_cnt_max`',
-              'resulting_object_cnt_max test')
+    local result = gql_query:execute(variables)
+    assert(result.data == nil, "this test should fail")
+    assert(result.errors ~= nil, "this test should fail")
+    local err = test_utils.strip_error(result.errors[1].message)
+    test:like(err,
+        'count%[4%] exceeds limit%[3%] %(`resulting_object_cnt_max`',
+        'resulting_object_cnt_max test')
+
     variables = {
         user_id = 5,
         description = "no such description"
     }
-    ok, result = pcall(gql_query.execute, gql_query, variables)
-    assert(ok == false, "this test should fail")
-    test:like(result,
-              'count%[6%] exceeds limit%[5%] %(`fetched_object_cnt_max`',
-              'resulting_object_cnt_max test')
+    local result = gql_query:execute(variables)
+    assert(result.data == nil, "this test should fail")
+    assert(result.errors ~= nil, "this test should fail")
+    local err = test_utils.strip_error(result.errors[1].message)
+    test:like(err,
+        'count%[6%] exceeds limit%[5%] %(`fetched_object_cnt_max`',
+        'resulting_object_cnt_max test')
 
     assert(test:check(), 'check plan')
 end

test/common/mutation.test.lua

@@ -92,7 +92,7 @@ local function check_insert(test, gql_wrapper, virtbox, mutation_insert,
         -- check mutation result from graphql
         local result = gql_mutation_insert:execute(dont_pass_variables and {} or
             variables_insert)
-        test:is_deeply(result, exp_result_insert, 'insert result')
+        test:is_deeply(result.data, exp_result_insert, 'insert result')
         -- check inserted user
         local tuple = get_tuple(virtbox, 'user_collection', {user_id})
         test:ok(tuple ~= nil, 'tuple was inserted')
@@ -134,7 +134,7 @@ local function check_insert_order_metainfo(test, gql_wrapper, virtbox,
         -- check mutation result
         local gql_mutation_insert = gql_wrapper:compile(mutation_insert)
         local result = gql_mutation_insert:execute(variables)
-        test:is_deeply(result, exp_result_insert, 'insert result')
+        test:is_deeply(result.data, exp_result_insert, 'insert result')
 
         -- check inserted tuple
         local EXTERNAL_ID_STRING = 1 -- 0 is for int
@@ -218,7 +218,7 @@ local function check_update(test, gql_wrapper, virtbox, mutation_update,
         -- check mutation result from graphql
         local result = gql_mutation_update:execute(dont_pass_variables and {} or
             variables_update)
-        test:is_deeply(result, exp_result_update, 'update result')
+        test:is_deeply(result.data, exp_result_update, 'update result')
         -- check updated user
         local tuple = get_tuple(virtbox, 'user_collection', {user_id})
         test:ok(tuple ~= nil, 'updated tuple exists')
@@ -282,7 +282,7 @@ local function check_update_order_metainfo(test, gql_wrapper, virtbox,
         -- check mutation result
         local gql_mutation_update = gql_wrapper:compile(mutation_update)
         local result = gql_mutation_update:execute(variables)
-        test:is_deeply(result, exp_result_update, 'update result')
+        test:is_deeply(result.data, exp_result_update, 'update result')
 
         -- check updated tuple
         local tuple = get_tuple(virtbox, 'order_metainfo_collection',
@@ -334,7 +334,7 @@ local function check_delete(test, gql_wrapper, virtbox, mutation_delete,
         -- check mutation result from graphql
         local result = gql_mutation_delete:execute(dont_pass_variables and {} or
             variables_delete)
-        test:is_deeply(result, exp_result_delete, 'delete result')
+        test:is_deeply(result.data, exp_result_delete, 'delete result')
 
         -- check the user was deleted
         local tuple = get_tuple(virtbox, 'user_collection', {user_id})
@@ -468,10 +468,10 @@ local function run_queries(gql_wrapper, virtbox, meta)
         }
     ]]
     local gql_mutation_insert_3i = gql_wrapper:compile(mutation_insert_3i)
-    local ok, err = pcall(gql_mutation_insert_3i.execute,
-        gql_mutation_insert_3i, {})
+    local result = gql_mutation_insert_3i:execute({})
+    local err = test_utils.strip_error(result.errors[1].message)
     local err_exp = '"insert" must be the only argument when it is present'
-    test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp},
+    test:is(err, err_exp,
         '"insert" argument is forbidden with other filters (object arguments)')
 
     -- test "insert" argument is forbidden with list arguments
@@ -489,10 +489,10 @@ local function run_queries(gql_wrapper, virtbox, meta)
         }
     ]]
     local gql_mutation_insert_4i = gql_wrapper:compile(mutation_insert_4i)
-    local ok, err = pcall(gql_mutation_insert_4i.execute,
-        gql_mutation_insert_4i, {})
+    local result = gql_mutation_insert_4i:execute({})
+    local err = test_utils.strip_error(result.errors[1].message)
     local err_exp = '"insert" must be the only argument when it is present'
-    test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp},
+    test:is(err, err_exp,
         '"insert" argument is forbidden with other filters (list arguments)')
 
     -- test "insert" argument is forbidden with other extra argument
@@ -510,10 +510,10 @@ local function run_queries(gql_wrapper, virtbox, meta)
         }
     ]]
     local gql_mutation_insert_5i = gql_wrapper:compile(mutation_insert_5i)
-    local ok, err = pcall(gql_mutation_insert_5i.execute,
-        gql_mutation_insert_5i, {})
+    local result = gql_mutation_insert_5i:execute({})
+    local err = test_utils.strip_error(result.errors[1].message)
     local err_exp = '"insert" must be the only argument when it is present'
-    test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp},
+    test:is(err, err_exp,
         '"insert" argument is forbidden with other filters (extra arguments)')
 
     -- test inserting an object into a collection with subrecord, union, array
@@ -1134,11 +1134,11 @@ local function run_queries(gql_wrapper, virtbox, meta)
             user_id = 'user_id_201',
         }
     }
-    local ok, err = pcall(gql_mutation_update_4.execute, gql_mutation_update_4,
-        variables_update_4)
+    local result = gql_mutation_update_4:execute(variables_update_4)
+    local err = test_utils.strip_error(result.errors[1].message)
     local err_exp = "Attempt to modify a tuple field which is part of index " ..
         "'user_id_index' in space 'user_collection'"
-    test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp},
+    test:is(err, err_exp,
         'updating of a field of a primary key when it is NOT shard key field')
 
     local mutation_update_5 = [[
@@ -1159,11 +1159,11 @@ local function run_queries(gql_wrapper, virtbox, meta)
             order_id = 'order_id_4001',
         }
     }
-    local ok, err = pcall(gql_mutation_update_5.execute, gql_mutation_update_5,
-        variables_update_5)
+    local result = gql_mutation_update_5:execute(variables_update_5)
+    local err = test_utils.strip_error(result.errors[1].message)
     local err_exp = "Attempt to modify a tuple field which is part of index " ..
         "'order_id_index' in space 'order_collection'"
-    test:is_deeply({ok, test_utils.strip_error(err)}, {false, err_exp},
+    test:is(err, err_exp,
         'updating of a field of a primary key when it is shard key field')
 
     -- }}}

test/common/pcre.test.lua

@@ -50,7 +50,7 @@ local function run_queries(gql_wrapper)
           middle_name: Ivanovich
     ]]):strip())
 
-    test:is_deeply(result_1_1, exp_result_1_1, '1_1')
+    test:is_deeply(result_1_1.data, exp_result_1_1, '1_1')
 
     -- }}}
     -- {{{ offset + regexp match
@@ -71,7 +71,7 @@ local function run_queries(gql_wrapper)
           first_name: Vasiliy
     ]]):strip())
 
-    test:is_deeply(result_1_2, exp_result_1_2, '1_2')
+    test:is_deeply(result_1_2.data, exp_result_1_2, '1_2')
 
     -- }}}
     -- {{{ UTF-8 in regexp
@@ -93,7 +93,7 @@ local function run_queries(gql_wrapper)
           middle_name: Иванович
     ]]):strip())
 
-    test:is_deeply(result_1_3, exp_result_1_3, '1_3')
+    test:is_deeply(result_1_3.data, exp_result_1_3, '1_3')
 
     -- }}}
 
@@ -120,7 +120,7 @@ local function run_queries(gql_wrapper)
         return gql_query_1i:execute({})
     end)
 
-    test:is_deeply(result_1i_1, exp_result_1_1, '1i_1')
+    test:is_deeply(result_1i_1.data, exp_result_1_1, '1i_1')
 
     -- }}}
 
@@ -154,7 +154,8 @@ local function run_queries(gql_wrapper)
         return gql_query_2:execute({})
     end)
 
-    test:is_deeply(result_2, exp_result_2, 'regexp match by a subrecord field')
+    test:is_deeply(result_2.data, exp_result_2,
+        'regexp match by a subrecord field')
 
     -- }}}
 
@@ -166,6 +167,10 @@ box.cfg({})
 
 test_utils.run_testdata(testdata, {
     run_queries = run_queries,
+    graphql_opts = {
+        -- gh-137: timeout exceeded
+        timeout_ms = 10000, -- 10 seconds
+    }
 })
 
 os.exit()

test/common/query_timeout.test.lua

@@ -7,7 +7,7 @@ package.path = fio.abspath(debug.getinfo(1).source:match("@?(.*/)")
     :gsub('/./', '/'):gsub('/+$', '')) .. '/../../?.lua' .. ';' .. package.path
 
 local tap = require('tap')
-local utils = require('test.test_utils')
+local test_utils = require('test.test_utils')
 local testdata = require('test.testdata.user_order_item_testdata')
 
 local function run_queries(gql_wrapper)
@@ -30,18 +30,19 @@ local function run_queries(gql_wrapper)
     ]]
 
     local gql_query = gql_wrapper:compile(query)
-    local variables = {
-    }
-    local ok, result = pcall(gql_query.execute, gql_query, variables)
-    assert(ok == false, 'this test should fail')
-    test:like(result, 'query execution timeout exceeded', 'timeout test')
+    local variables = {}
+    local result = gql_query:execute(variables)
+    assert(result.data == nil, "this test should fail")
+    assert(result.errors ~= nil, "this test should fail")
+    local err = test_utils.strip_error(result.errors[1].message)
+    test:like(err, 'query execution timeout exceeded', 'timeout test')
 
     assert(test:check(), 'check plan')
 end
 
 box.cfg({})
 
-utils.run_testdata(testdata, {
+test_utils.run_testdata(testdata, {
     run_queries = run_queries,
     graphql_opts = {
         timeout_ms = 0.001,

test/extra/to_avro_arrays.test.lua

@@ -96,16 +96,16 @@ user_collection:
 ]]
 result_expected = yaml.decode(result_expected)
 local result = gql_query:execute(variables)
-test:is_deeply(result, result_expected, 'graphql query exec result')
+test:is_deeply(result.data, result_expected, 'graphql query exec result')
 local ok, ash, r, fs, _
 ok, ash = avro.create(avros)
 assert(ok)
-ok, _ = avro.validate(ash, result)
+ok, _ = avro.validate(ash, result.data)
 assert(ok)
 test:is(ok, true, 'gql result validation by avro')
 ok, fs = avro.compile(ash)
 assert(ok)
-ok, r = fs.flatten(result)
+ok, r = fs.flatten(result.data)
 assert(ok)
 ok, r = fs.unflatten(r)
 assert(ok)