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

ER: Provide the ability to customize reply error values #854

Closed
nathklei opened this Issue Apr 10, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@nathklei
Copy link
Contributor

commented Apr 10, 2019

It would be nice if I had the ability to set my own error codes that are specific to my organization. Right now it seems that anything I would set in e.g. my security policy would get overridden with the default in:
https://github.com/cometd/cometd/blob/4.0.x/cometd-java/cometd-java-server/src/main/java/org/cometd/server/BayeuxServerImpl.java#L1193-L1198

If possible we could skip the put if the key existed already (e.g. putIfAbsent)

I could use a different field, possibly ext, though I'd prefer using the standard error field for clarity. It could be that the existing error codes are there for a reason, and overriding them could cause unintentional issues that I'm not aware of - or that deviating from the format defined in https://docs.cometd.org/current/reference/#_error is inadvisable, though the language does use MAY/SHOULD and not MUST.

Thank you for the help!

@sbordet

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

@nathklei I'm a little hesitant to allow applications to change the Bayeux error field.

You have 2 alternatives that should work well for you:

  • Add another field to the message, say app_error or failure so that your message looks like this:
{
  "error": "403::subscription_invalid",
  "failure": {
    "description": "my_custom_error",
    "context": ...
  }
  ...
}
  • Use the data field of the message to convey the failure.

Note that you can access the reply message from the SecurityPolicy in this way:
https://docs.cometd.org/current/reference/#_concepts_application_authentication

Would either work for you?

@nathklei

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Hey @sbordet thanks for the response. I understand that allowing customizations like this isn't something you'd like to allow. I'd already started going down a path of using my own fields for serializing additional error info - I'll continue going down that path which works well as I have a well-defined payload I can include. Going to close this one out!

@nathklei nathklei closed this Apr 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.