[WIP] Correct HTTP response status for subscription
Signed-off-by: Deepanshu Agarwal [email protected]
Description
This intends to fix n/ack of http response status correctly during subscription, as per https://docs.dapr.io/reference/api/pubsub_api/#expected-http-response
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #333
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:
- [x] Code compiles correctly
- [ ] Created/updated tests
- [ ] Extended the documentation
Codecov Report
Merging #334 (ce8416b) into main (d3f2bc0) will increase coverage by
9.34%. The diff coverage is66.66%.
@@ Coverage Diff @@
## main #334 +/- ##
==========================================
+ Coverage 25.43% 34.78% +9.34%
==========================================
Files 76 77 +1
Lines 6164 6170 +6
Branches 205 205
==========================================
+ Hits 1568 2146 +578
+ Misses 4592 4005 -587
- Partials 4 19 +15
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/implementation/Server/HTTPServer/pubsub.ts | 17.64% <25.00%> (+5.14%) |
:arrow_up: |
| src/enum/SubscribedMessageHttpResponse.enum.ts | 100.00% <100.00%> (ø) |
|
| src/implementation/Client/GRPCClient/state.ts | 5.00% <0.00%> (+1.00%) |
:arrow_up: |
| .../implementation/Client/GRPCClient/configuration.ts | 7.27% <0.00%> (+1.81%) |
:arrow_up: |
| src/proto/dapr/proto/runtime/v1/dapr_pb.js | 30.68% <0.00%> (+2.57%) |
:arrow_up: |
| src/implementation/Client/GRPCClient/lock.ts | 16.00% <0.00%> (+4.00%) |
:arrow_up: |
| src/utils/Settings.util.ts | 70.83% <0.00%> (+4.16%) |
:arrow_up: |
| src/implementation/Client/GRPCClient/metadata.ts | 16.66% <0.00%> (+4.16%) |
:arrow_up: |
| src/implementation/Client/GRPCClient/secret.ts | 13.63% <0.00%> (+4.54%) |
:arrow_up: |
| src/proto/dapr/proto/runtime/v1/dapr_grpc_pb.js | 10.85% <0.00%> (+6.85%) |
:arrow_up: |
| ... and 22 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Thank you @DeepanshuA, this was really fast!
Quick question - were you able to test the existing behaviour? Also it would be great to add e2e tests for this if possible, or we can create an issue to track it separately.
Thank you @DeepanshuA, this was really fast!
Quick question - were you able to test the existing behaviour? Also it would be great to add e2e tests for this if possible, or we can create an issue to track it separately.
Yeah, I was able to test the existing behavior and after this change. Earlier, it always landed in runtime with appResponse.Status being blank ("") https://github.com/dapr/dapr/blob/91af1367f29d77f16f8acee1cfff53830bc42b63/pkg/runtime/runtime.go#L1719. But, now it lands either with status: "SUCCESS" or status: "RETRY".
I have created a separate issue for adding e2e test: https://github.com/dapr/js-sdk/issues/335