symbol / catapult-rest

Catapult REST combines HTTP and WebSockets to perform read and write actions on the blockchain
https://docs.symbolplatform.com/api.html
43 stars 32 forks source link

When REST fails to notify the client when unable to submit a transaction. #563

Open Wayonb opened 3 years ago

Wayonb commented 3 years ago

When REST fails to submit a transaction and the their is a websocket register the submitter of the transaction should get notify.

rest-gateway_1  | verbose: websocket Y3YFHID7JOWQR6JA2SOFF332UVSU55PW: created /ws websocket connection {"timestamp":"2021-01-30T20:40:33.365Z"}
db_1            | {"t":{"$date":"2021-01-30T20:40:36.010+00:00"},"s":"I",  "c":"COMMAND",  "id":51803,   "ctx":"conn25","msg":"Slow query","attr":{"type":"command","ns":"catapult.namespaces","command":{"find":"namespaces","filter":{"$and":[{"namespace.alias.mosaicId":{"$in":[3239218328380109726]}},{"namespace.alias.type":1},{"$and":[{"meta.latest":true},{"$or":[{"namespace.endHeight":-1},{"namespace.endHeight":{"$gt":64423}}]}]}]},"returnKey":false,"showRecordId":false,"lsid":{"id":{"$uuid":"bb1bc086-ecd2-4288-a8dc-4badb5fb5321"}},"$db":"catapult"},"planSummary":"IXSCAN { meta.latest: -1, meta.index: 1, namespace.level0: 1, namespace.depth: 1 }","keysExamined":413639,"docsExamined":413639,"cursorExhausted":true,"numYields":427,"nreturned":1,"queryHash":"726729D0","planCacheKey":"B9C490C5","reslen":399,"locks":{"ReplicationStateTransition":{"acquireCount":{"w":428}},"Global":{"acquireCount":{"r":428}},"Database":{"acquireCount":{"r":428}},"Collection":{"acquireCount":{"r":428}},"Mutex":{"acquireCount":{"r":1}}},"storage":{"data":{"bytesRead":115377231,"timeReadingMicros":504780}},"protocol":"op_msg","durationMillis":1802}}
rest-gateway_1  | error: caught error 503 connection failed {"jse_shortmsg":"connection failed","jse_info":{},"body":{"code":"ServiceUnavailable","message":"connection failed"},"stack":"ServiceUnavailableError: connection failed\n    at Object.createServiceUnavailableError (/app/catapult-rest/rest/_build/server/errors.js:56:44)\n    at TLSSocket.<anonymous> (/app/catapult-rest/rest/_build/connection/connectionService.js:104:20)\n    at TLSSocket.emit (events.js:327:22)\n    at net.js:674:12\n    at TCP.done (_tls_wrap.js:567:7)","timestamp":"2021-01-30T20:40:36.472Z"}
fboucquez commented 3 years ago

The submitter will get an error synchronously when calling the rest announce endpoint, not via the listener. Why should this submitter or any other submitter client receive an error via listener?

Currently, rest is just a proxy between server message queues and the clients. This would require for rest to create and notify its own messages.

rg911 commented 3 years ago

Yeah, transaction announcement vai the put request in the endpoint, and if errors, the rest returns other error codes rather than 202

fboucquez commented 3 years ago

So the client is notified with the response error. Nothing to do here?

Wayonb commented 3 years ago

OK. @rg911 Does this means the wallet/Faucet is not checking this error then?

rg911 commented 3 years ago

the error of websocket connection does get thrown in the wallet. But it only gets thrown when calling the server API, e.g. announce transactions / node info etc... Wallet won't get notified when the ws listener is forcibly closed on the server side

rg911 commented 3 years ago

rest does notify clients when node connection fails.

image

rg911 commented 3 years ago

can we close this issue?