vertx-web icon indicating copy to clipboard operation
vertx-web copied to clipboard

Inconsistent structure of error messages in EventBusBridge

Open EmadAlblueshi opened this issue 11 months ago • 7 comments

IMHO the structure for the type of error messages should be consistent. The following illustrates inconsistency

https://github.com/vert-x3/vertx-web/blob/1cd9a5cd2f98ffc59ea600326b34e8a696ef460d/vertx-web/src/main/java/io/vertx/ext/web/handler/sockjs/impl/EventBusBridgeImpl.java#L339

https://github.com/vert-x3/vertx-web/blob/1cd9a5cd2f98ffc59ea600326b34e8a696ef460d/vertx-web/src/main/java/io/vertx/ext/web/handler/sockjs/impl/EventBusBridgeImpl.java#L494

https://github.com/vert-x3/vertx-web/blob/1cd9a5cd2f98ffc59ea600326b34e8a696ef460d/vertx-web/src/main/java/io/vertx/ext/web/handler/sockjs/impl/EventBusBridgeImpl.java#L594

Currently, sometimes the content of the error message inside "body" and sometimes inside "message" and we should have unified structure for the type of error messages.

EmadAlblueshi avatar Dec 22 '24 06:12 EmadAlblueshi

@EmadAlblueshi any proposal to make in this regard? Always use JSON?

tsegismont avatar Jan 06 '25 16:01 tsegismont

Hi @tsegismont

Any error message should be as follows :

{
  "type" : "err",
  "failureType" : "[the exception name like 'socketexception']",
  "message" : "[the message content as 'String']"
}

The above currently implemented but in the EventBusBridge implementation there are "some" error messages as follows :

{
  "type" : "err",
  "body" : "[the message content as 'String']"
}

My proposal is to unify the first structure in all error messages.

EmadAlblueshi avatar Jan 07 '25 12:01 EmadAlblueshi

@tsegismont In addition, this should be done to vertx-eventbus.js too.

EmadAlblueshi avatar Jan 07 '25 12:01 EmadAlblueshi

Would you mind contributing this?

tsegismont avatar Jan 16 '25 17:01 tsegismont

Hi @tsegismont

Sure, by next week and this would be planned for Vert.x 5 only as @vietj said because in 4.x it will be a breaking change.

In addition, we should review the existing behavior too probably needs some enhancements if needed.

EmadAlblueshi avatar Jan 16 '25 17:01 EmadAlblueshi

If you can do it early next week it should be fine, we are waiting for a new Netty 4.2 candidate release

tsegismont avatar Jan 16 '25 17:01 tsegismont

@tsegismont I will do my best!

EmadAlblueshi avatar Jan 16 '25 17:01 EmadAlblueshi