You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ponymail.apache.org by sebbASF <gi...@git.apache.org> on 2016/09/29 12:12:30 UTC
[GitHub] incubator-ponymail issue #162: Bug: pcall() idiom to protect against elastic...
GitHub user sebbASF opened an issue:
https://github.com/apache/incubator-ponymail/issues/162
Bug: pcall() idiom to protect against elastic.lua exceptions is flawed
The elastic.lua wrapper module can throw an error for some valid invocations - e.g. document not found.
The way this is handled currently is as follows:
local _, doc = pcall(function() return elastic.get("mbox", eid or "hmm") end)
if not doc or not doc.subject then ... end
However, if elastic.get() throws an error, the doc variable will be the error message string.
AFAICT it's not possible for doc to be false/nil currently, so it's not useful as a check.
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ponymail issue #162: Bug: pcall() idiom to protect against elastic...
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on the issue:
https://github.com/apache/incubator-ponymail/issues/162
Further checks show that some code does not check if the returned doc is valid after calling elastic.get()
A better solution would be to add an optional get parameter; if specified then it will return for 404. Then only code that is expecting a 404 will ever see one.
The alternative is to always use pcall, but that catches all errors.
It would be better to let these get reported, rather than assuming there has been a 404.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ponymail issue #162: Bug: pcall() idiom to protect against elastic...
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on the issue:
https://github.com/apache/incubator-ponymail/issues/162
Agreed, elastic should only throw an error for real errors; that would simplify the code as the pcall() wrapper would not be needed at all.
AFAICT the code only works because errors return a string so references such as doc.subject will be false.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ponymail issue #162: Bug: pcall() idiom to protect against elastic...
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on the issue:
https://github.com/apache/incubator-ponymail/issues/162
It looks as though 404s are only to be expected from elastic.get.
So it might make sense to only add a check for 404 when the caller is get.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ponymail issue #162: Bug: pcall() idiom to protect against elastic...
Posted by Humbedooh <gi...@git.apache.org>.
Github user Humbedooh commented on the issue:
https://github.com/apache/incubator-ponymail/issues/162
I think the main issue here is, elastic shouldn't error out on a 404, only other 4xx/5xx errors.
it should be changed to uhm... something like "if code != 200 and code != 404" in there...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ponymail issue #162: Bug: pcall() idiom to protect against elastic...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the issue at:
https://github.com/apache/incubator-ponymail/issues/162
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---