Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arity errors #2463

Open
neumino opened this issue May 24, 2014 · 4 comments
Open

Arity errors #2463

neumino opened this issue May 24, 2014 · 4 comments
Assignees
Milestone

Comments

@neumino
Copy link
Member

neumino commented May 24, 2014

For

r.table("test").zip(1)

The server will send back this error:

RqlCompileError: Expected 1 argument(s) but found 2. in:
r.db("test").table("test").zip(1)

The error we send back is not super-friendly because

  • There is no backtrace
  • We do not report the name of the function that was called with the wrong number of arguments
  • The server suppose that everything uses the notation r.zip(r.table('test'), 1)

About the last point, r.and is a valid syntax, so we can't just suppose that we always chain methods. But if the function that triggered the error was reported, the driver could catch the error and update the number of arguments depending of the method.

Putting in polish.

@neumino neumino added this to the 1.13-polish milestone May 24, 2014
@neumino
Copy link
Member Author

neumino commented May 29, 2014

Actually, we could have one term for r.add and one for datum.add.
That way:

  • it solve the problem of the number of arguments
  • we can build better backtraces
  • it would avoid strange things like r.add(1)

Same thing for the other commands, especially for tableCreate, tableDrop and table.

@mlucy
Copy link
Member

mlucy commented May 29, 2014

That sounds really bad to me. The terms sent to the server should represent an AST, which should be independent of what people typed to produce it. We'd end up having two of every term, and some languages (like Clojure or C) would only ever send half of them, then on the server we'd turn them into one "real" term.

I would rather make the server return structured error messages ({type: 'type_error', term: 'add', arg: '3', message: ..., bt: ...}) so that drivers can choose to render them in driver-specific ways (since the driver knows whether you were using infix or postfix notation to generate the AST).

@coffeemug
Copy link
Contributor

I would rather make the server return structured error messages ({type: 'type_error', term: 'add', arg: '3', message: ..., bt: ...}) so that drivers can choose to render them in driver-specific ways (since the driver knows whether you were using infix or postfix notation to generate the AST).

👍

@neumino
Copy link
Member Author

neumino commented May 29, 2014

r.add(1) and r.expr(1).add() compile to the same query.

So if the driver wants to make the distinction between the two notations, it has to build a second "query" that keeps track of the notation used, which is cumbersome to do.

That being said, I'm ok just sending back a structured error message for now. That would make errors way more readable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants