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

report text of malformed querries in Logcat #954

Closed

Conversation

matkoniecz
Copy link
Member

@matkoniecz matkoniecz commented Mar 7, 2018

removes need to create temporary debugging statements during development
currently with code causing 400 errors one must create temporary debug entry outputting failing query, now this will be done automatically

part of fixing 24ac256 that is part of fixing #920 that is part of fixing #685 :)

removes need to create temporary debugging statements during development
currently with code causing 400 errors one must create temporary debug entry outputting failing query, now this will be done automatically
@matkoniecz matkoniecz force-pushed the automatic_debug_for_400 branch from c0b052e to b1d6470 Compare March 7, 2018 13:59
@westnordost
Copy link
Member

OverpassMapDataDao is supposed to be not coupled with Android code. Using the Android log system would do that.
Also, I do not understand what you are trying to achieve here and how this solution would help (you), actually.

@matkoniecz
Copy link
Member Author

Also, I do not understand what you are trying to achieve here and how this solution would help (you), actually.

It helps if one has a bug in code that creates malformed overpass queries.

Such bugs causes Overpass to return 400 error code and 400 error code always indicates a bug.

Currently I always create a temporary debug entry that is outputting generated query - with this code it would be no longer necessary and query would be automatically printed to a log.

Using the Android log system would do that.

I was no aware about that, I will look for a better place to place this (if that is possible). Though I am curious why

OverpassMapDataDao is supposed to be not coupled with Android code

after all, this program will always run on Android - and logging should not have side effects (except logging).

@westnordost
Copy link
Member

westnordost commented Mar 8, 2018

I was thinking of putting that class into osmapi, actually. But in general, I want of course as least coupling as possible.

So if you want to debug possibly wrong Overpass QL strings, why is the current behavior - that it crashes the app and thus prints the stack trace - not sufficient?

@matkoniecz
Copy link
Member Author

It is only giving me info that query is malformed and not showing query itself.

Showing query that was generated (and was malformed) is very often useful to pinpoint the bug and see what went wrong.

@matkoniecz
Copy link
Member Author

I was thinking of putting that class into osmapi, actually.

I will look at it.

@westnordost
Copy link
Member

westnordost commented Mar 9, 2018 via email

@matkoniecz
Copy link
Member Author

matkoniecz commented Mar 12, 2018

I will submit an updated version once I again create code that requires debugging due to 400 error code.

@matkoniecz
Copy link
Member Author

@westnordost

new variant of that: what you think about matkoniecz@76fc967 ? Would it be also be rejected due to unclean architecture? (String in StringWithCursor becomes public)

This just allowed me to track down irritating and silly bug in the query syntax

@westnordost
Copy link
Member

Yes, likely to be rejected. An exception does not usually include the object itself (in this case a string).

What bug did you track down?

@matkoniecz
Copy link
Member Author

query starting from """"nodes, ways

(multiline string that got fourth " character)

@westnordost
Copy link
Member

Shouldn't it fail with a message like "yadda yadda yadda at cursor position 0" or something similar?

@matkoniecz
Copy link
Member Author

oh it failed: but I was unable to spot it (syntax highlighting could be smarter instead of making it entirely green). I have simply looked at """" and seen """

And error message about parsing failure at position 0 of "nodes, ways was much more obvious to interpret

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

Successfully merging this pull request may close these issues.

2 participants