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.

Sign up for GitHub

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

Jump to bottom

reduce with an inappropriate function throws the wrong error #4189

Closed
neumino opened this issue May 6, 2015 · 6 comments
Closed

reduce with an inappropriate function throws the wrong error #4189

neumino opened this issue May 6, 2015 · 6 comments
Labels
good_for_contributors
Milestone
2.2

Comments

@neumino
Copy link
Member

neumino commented May 6, 2015 

r.expr([1,2,3]).reduce(function(left) {
  return left
})
// Throws
RqlRuntimeError: Expected 1 argument but found 2 in:
// Instead of `RqlRuntimeError: Expected 2 arguments but found 1 in:`

r.expr([1,2,3]).reduce(function(left, right, extra) {
  return left
})
//Throws
RqlRuntimeError: Expected 3 arguments but found 2 in:
// Instead of `RqlRuntimeError: Expected 2 arguments but found 3 in:`
@neumino
Copy link
Member Author

neumino commented May 6, 2015

The same happens with filter (and I guess other methods?)

@mlucy
Copy link
Member

mlucy commented May 6, 2015

What's the expected error?

@Tryneus
Copy link
Member

Tryneus commented May 6, 2015

@mlucy, the printf parameters are switched. Or the error is being thrown when the function is called rather than when it is created.

@danielmewes
Copy link
Member

The current error messages make sense to me. I don't think they are wrong.

As an improvement, we could check the arity of functions before we call them, and then print something like "Expected a 2-argument function but found a 3-argument function in:..."

@danielmewes danielmewes added this to the backlog milestone May 6, 2015
@mlucy
Copy link
Member

mlucy commented May 6, 2015

My interpretation of the current error message is "This function expected 3 arguments but was passed 2." We should probably change the message to be clearer if it confused two people.

@danielmewes danielmewes modified the milestones: subsequent, backlog May 6, 2015
@danielmewes danielmewes modified the milestones: backlog, subsequent Jun 17, 2015
@zhenchaoli zhenchaoli mentioned this issue Aug 7, 2015
Merged
@mlucy
Copy link
Member

mlucy commented Aug 15, 2015

This was fixed by @fantasticsid in PR #4644 (reviewed by me).

@mlucy mlucy closed this as completed Aug 15, 2015
@mlucy mlucy modified the milestones: 2.2-polish, backlog Aug 15, 2015
@danielmewes danielmewes modified the milestones: 2.2-polish, 2.2 Oct 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good_for_contributors
Projects
None yet
Milestone
2.2
Development

No branches or pull requests
4 participants
@danielmewes @Tryneus @neumino @mlucy