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

data explorer does not correctly handle unexpected token errors #1334

Closed
wmrowan opened this issue Aug 20, 2013 · 7 comments
Closed

data explorer does not correctly handle unexpected token errors #1334

wmrowan opened this issue Aug 20, 2013 · 7 comments
Assignees
@neumino
Labels
tp:bug
Milestone
1.10.0

Comments

@wmrowan
Copy link
Contributor

wmrowan commented Aug 20, 2013

We emit an error on the connection when an unexpected token is received on the connection. In this case the data explorer pops up an error message saying that the connection to the database was lost. This is incorrect. The connection remains usable.
@ghost ghost assigned neumino Aug 20, 2013
@neumino
Copy link
Member

neumino commented Aug 20, 2013

How do you have the connection emit such error?

@wmrowan
Copy link
Contributor Author

wmrowan commented Aug 20, 2013

This is a relatively recent change. See https://github.com/rethinkdb/rethinkdb/blob/next/drivers/javascript/net.coffee#L184

@coffeemug
Copy link
Contributor

What would the user have to do to reproduce this error?

@neumino
Copy link
Member

neumino commented Aug 21, 2013

The user probably have to send a protobuf that the server cannot parse -- #1248
I'm not sure the user can easily reproduce this error.

For my side, I'll just call _processResponse with a non existing token to trigger the error.

@neumino
Copy link
Member

neumino commented Aug 21, 2013

This query is enough to trigger the error:

r.expr(1).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1)).add(r.expr(1))

neumino pushed a commit that referenced this issue Aug 21, 2013
Handle unexpected token -- Related to #1334
a5dfb31
@neumino
Copy link
Member

neumino commented Aug 21, 2013

Branch michel_1334_unexpected_token
In review 867 assigned to @mglukhovsky

neumino pushed a commit that referenced this issue Aug 22, 2013
Fix for error listener + typo -- Related to #1334
c9953b7
@neumino
Copy link
Member

neumino commented Aug 30, 2013

Review done.
Merged in next as 8eaf37a

@neumino neumino closed this as completed Aug 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@neumino neumino

Labels
tp:bug
Projects
None yet
Milestone
1.10.0
Development

No branches or pull requests
3 participants
@coffeemug @wmrowan @neumino