Skip to content

Fixed issue where websocket hook did not call original completion#609

Merged
NSExceptional merged 1 commit into
FLEXTool:masterfrom
ogres:fix/websocket-message-send-hook
Aug 10, 2022
Merged

Fixed issue where websocket hook did not call original completion#609
NSExceptional merged 1 commit into
FLEXTool:masterfrom
ogres:fix/websocket-message-send-hook

Conversation

@ogres

@ogres ogres commented Jul 14, 2022

Copy link
Copy Markdown
Contributor

Send message hook was changing original completion instead of wrapping it in a hook.
Similar hook already exists for receivemessage functionality, but it was not working for send-message, original completion was never called.
Adjusted code to create a new completionHook which records the information and then original completion block.

I also updated API-Key for piesocket and added a message after sending message, to make sure that completion is being called and at least success/failure message is logged.

@ogres

ogres commented Aug 8, 2022

Copy link
Copy Markdown
Contributor Author

@NSExceptional could you have a look at this one?

Comment thread Example/FLEXample/MiscNetworkRequests.m Outdated
Comment thread Classes/Network/PonyDebugger/FLEXNetworkObserver.m Outdated

@NSExceptional NSExceptional left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, thank you! Also, sorry, I must have totally missed this PR. Thanks for pinging me.

Comment thread Classes/Network/PonyDebugger/FLEXNetworkObserver.m Outdated
@ogres

ogres commented Aug 10, 2022

Copy link
Copy Markdown
Contributor Author

Nice catch, thank you! Also, sorry, I must have totally missed this PR. Thanks for pinging me.

I've added nullability check
Thanks for the review 👍

@NSExceptional
NSExceptional merged commit 1b91bc9 into FLEXTool:master Aug 10, 2022
@ogres
ogres deleted the fix/websocket-message-send-hook branch August 10, 2022 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants