Consensys / mythx-developer-support

Support resources and bug reporting for Mythril Tool Developers
3 stars 1 forks source link

HTTP 200 return code on errored analysis #13

Closed rocky closed 5 years ago

rocky commented 6 years ago
$ ./analyses-status.sh a1aaa0c9-6241-4956-aac8-a0acd4617156
Issuing HTTP GET https://api.mythril.ai/v1/analyses/a1aaa0c9-6241-4956-aac8-a0acd4617156
  (with MYTHRIL_API_KEY)
curl completed sucessfully. Output follows...
HTTP/1.1 200 OK
{
  "apiVersion": "v1.0.25",
  "error": "An error occurred when running myth.\nOutput: \nError Output: Traceback (most recent call last):\n  File \"/usr/local/bin/myth\", line 9, in <module>\n    mythril.interfaces.cli.main()\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/interfaces/cli.py\", line 206, in main\n    max_depth=args.max_depth, execution_timeout=args.execution_timeout)\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/mythril.py\", line 380, in fire_lasers\n    max_depth=max_depth, execution_timeout=execution_timeout)\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/analysis/symbolic.py\", line 30, in __init__\n    self.laser.sym_exec(address)\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/laser/ethereum/svm.py\", line 61, in sym_exec\n    transaction.run(self.open_states, self)\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/laser/ethereum/transaction.py\", line 58, in run\n    evm.exec()\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/laser/ethereum/svm.py\", line 71, in exec\n    new_states, op_code = self.execute_state(global_state)\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/laser/ethereum/svm.py\", line 97, in execute_state\n    new_global_states = Instruction(op_code, self.dynamic_loader).evaluate(global_state)\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/laser/ethereum/instructions.py\", line 69, in evaluate\n    return instruction_mutator(global_state)\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/laser/ethereum/instructions.py\", line 34, in wrapper\n    new_global_states = func(self, global_state_copy)\n  File \"/usr/local/lib/python3.6/dist-packages/mythril/laser/ethereum/instructions.py\", line 77, in push_\n    value = BitVecVal(int(global_state.get_current_instruction()['argument'][2:], 16), 256)\nValueError: invalid literal for int() with base 16: ''\n",
  "mythrilVersion": "v0.18.11",
  "queueTime": 19,
  "runTime": 729,
  "status": "Error",
  "submittedAt": "2018-09-13T19:04:12.933Z",
  "submittedBy": "85484509-a281-4f4e-9cef-08fdd3c6fae2",
  "uuid": "a1aaa0c9-6241-4956-aac8-a0acd4617156"
}

200 OK? Wouldn't 201 or something different be better?

Continuing...

$ ./analyses-results.sh a1aaa0c9-6241-4956-aac8-a0acd4617156
curl completed sucessfully. Output follows...
{
  "status": 400,
  "error": "Analysis is not Finished"
}

Analysis is not finished? (Note that should either be in quotes, or better a lowercase "finished"). But status says it errored, so it is finished.

birdofpreyru commented 6 years ago

Regarding the first point:

In REST API HTTP status tells about the result of this very request. In your case, GET https://api.mythril.ai/v1/analyses/a1aaa0c9-6241-4956-aac8-a0acd4617156 - get information about analysis a1aaa0c9-6241-4956-aac8-a0acd4617156, which you got successfully, thus 200 OK. Sure, the status of the analysis itself is given by status field in the response.

HTTP 201 status is intended for things like creation of new users, etc.; but it is only useful when there is an endpoint that can create a new resource in some cases, and to not create it in other (and both are valid behaviors), thus using 201 for the first case and other status for the second allows to distinguish them. We, probably, can use 200 and 202 HTTP status to distinguish scenarious when analysis is queued, and when the result is taken from the cache right away, that might be conveninient for the client.

Regarding the error, I hopefully check this week. To clean-up error messaging is on my todo list already (#12)

rocky commented 5 years ago

Resolved.