ApeWorX / hosted-compiler

Compiler tool supporting Remix Online IDE
Apache License 2.0
4 stars 1 forks source link

Make errors more structured #50

Closed fubuloubu closed 1 month ago

fubuloubu commented 2 months ago

Describe

Downstream integrators want better support for displaying errors from this service, create a more structured way for displaying compiler errors so that they can be displayed

Specification

Remix's error structure is as follows:

interface CompilerError {
    status: string = "failed"; // NOTE: We don't really need to specify this as anything else
    message: string; // Use `e.message`
    column: number;
    line: number;
    errorType: string; // Use `e.__class__.__name__`
}

Not all errors would it make sense to change, but the result obtained from the /errors/{task_id} endpoint should be described this way, for any configuration or compile-time exceptions

Dependencies

n/a

joeizang commented 2 months ago

errorType can be SyntaxError Exception, which I think was a type of error I saw when testing.

fubuloubu commented 2 months ago

errorType can be SyntaxError Exception, which I think was a type of error I saw when testing.

right, but from your perspective it's just a string that gets printed right?

joeizang commented 2 months ago

Also the errors that are returned are very random and sometime has nothing to do with the code being compiled.

image

fubuloubu commented 2 months ago

Also the errors that are returned are very random and sometime has nothing to do with the code being compiled.

image

We are aware of that as a separate issue, if you use the proper PEP440 version specifier it should compile fine (or at least raise a proper compiler bug)

Ninjagod1251 commented 1 month ago

With the latest updated of Hosted Compiler #53

the exceptions/{task_id} now has a response like this:

[
  {
    "status": "failed",
    "message": "/tmp/tmpa059soqe/unknown-project-730365/contracts/fail_misspell.vy\nModuleNotFound:snekmate.tokens.ercasd20\n\n  contract \"/tmp/tmpa059soqe/unknown-project-730365/contracts/fail_misspell.vy:41\", line 41:0 \n       40 # @dev We import and initialise the `erc20` module.\n  ---> 41 from snekmate.tokens import ercasd20\n  --------^\n       42 initializes: erc20[ownable := ow]\n",
    "column": 0,
    "line": 41,
    "error_type": "VyperCompileError"
  }
]

message, line and column are different fields.

i would also note, it useful for remix to see the /tmp/tmpa059soqe/unknown-project-730365/contracts/fail_misspell.vy before the \n so you know what contract it fails on. But it would not be useful for the user.

you could pattern match to find the start of /tmp and end the match with \n like this

# Remove the /tmp/... part along with any trailing newline after it
 filtered_message = re.sub(r'/tmp[^:]*:\d+\n?', '', error_message)
fubuloubu commented 1 month ago

i would also note, it useful for remix to see the /tmp/tmpa059soqe/unknown-project-730365/contracts/fail_misspell.vy before the \n so you know what contract it fails on. But it would not be useful for the user.

you could pattern match to find the start of /tmp and end the match with \n like this

# Remove the /tmp/... part along with any trailing newline after it
 filtered_message = re.sub(r'/tmp[^:]*:\d+\n?', '', error_message)

This temporary file is due to how we are internally compiling it, I would suggest that we create a separate issue to find temporary directory (e.g. /tmp/.../unknown-project-.../contracts/) and remove it from any error text so that the user doesn't get confused