[Fix] Import <UIKit/UIWebView.h> for iOS + App crash when no handler for message from JS.

[ruslanskorb]

No description provided.

[marcuswestin]

Hi Ruslan,

WVJB intentionally crashes when a message is sent without a handler. It is the sort of bug that you want to discover quickly and easily - a crash makes the big obvious. I would take a PR that throws an exception instead, making it more explicit.
-- while mobile

On Tue, Aug 5, 2014 at 12:35 PM, Ruslan Skorb notifications@github.com
wrote:

You can merge this Pull Request by running:
git pull https://github.com/ruslanskorb/WebViewJavascriptBridge patch-1
Or you can view, comment on it, or merge it online at:
#97
-- Commit Summary --

• [Fix] Import <UIKit/UIWebView.h> for iOS.

• [Fix] App crash when no handler (_messageHandler) for message from JS.
  -- File Changes --
  M WebViewJavascriptBridge/WebViewJavascriptBridge.h (1)
  M WebViewJavascriptBridge/WebViewJavascriptBridge.m (4)
  -- Patch Links --
  https://github.com/marcuswestin/WebViewJavascriptBridge/pull/97.patch
  https://github.com/marcuswestin/WebViewJavascriptBridge/pull/97.diff

  Reply to this email directly or view it on GitHub:
  [Fix] Import <UIKit/UIWebView.h> for iOS + App crash when no handler for message from JS. #97

[ruslanskorb]

Hi Marcus,

Why do you think that the handler is required?

[ruslanskorb]

I think that the application can intentionally handle only registered handlers.

[marcuswestin]

Why would you ever want to send a request that can't possibly be handled?

If for example you mistype the name of your intended handler, it is beneficial that the app loudly helps you spot your error.
-- while mobile

On Wed, Aug 6, 2014 at 6:42 PM, Ruslan Skorb notifications@github.com
wrote:

I think that the application can intentionally handle only registered handlers.

Reply to this email directly or view it on GitHub:
#97 (comment)

[ruslanskorb]

I do not see any reason to send a request that can not possibly be handled.
If the developer mistyped the name of an intended handler then required event will not occur. He could understand what was going on using the logs.
I think that exceptions should be used as a last resort. For example, call overridden initializers of superclass, which must not be used in subclasses.

[marcuswestin]

Well, I definitely think failing early and failing loudly is a great principle, and based on my (very subjective) view of what's best for users of wvjb I intend to keep requiring a handler for messages that expect a handler. Though I do appreciate your PR, and accept that our opinions differ.

Thanks again for the PR!

Cheers,

Marcus 
-- while mobile

On Wed, Aug 6, 2014 at 7:32 PM, Ruslan Skorb notifications@github.com
wrote:

I do not see any reason to send a request that can not possibly be handled.
If the developer mistyped the name of an intended handler then required event will not occur. He could understand what was going on using the logs.

I think that exceptions should be used as a last resort. For example, call overridden initializers of superclass, which must not be used in subclasses.

Reply to this email directly or view it on GitHub:
#97 (comment)

[ruslanskorb]

OK. Thank you for your answers and for WVJB! This project is really cool!

Finally, I would like to demonstrate the use of NSException instead of NSLog -
https://github.com/ruslanskorb/WebViewJavascriptBridge/tree/no-handler-exception
(ruslanskorb@c4bc8bc, ruslanskorb@e8d83dc)

In this case WebKit automatically discards the exception and outputs the following message in the log:

*** WebKit discarded an uncaught exception in the webView:decidePolicyForNavigationAction:request:frame:decisionListener: delegate: <WVJBNoHandlerException> No handler for message from JS: <CFBasicHash 0x9b66310 [0x2587ec8]>{type = immutable dict, count = 2,
entries =>
    0 : <CFString 0x9b7ee60 [0x2587ec8]>{contents = "data"} = <CFString 0x9b65eb0 [0x2587ec8]>{contents = "Hello from JS button"}
    1 : <CFString 0x9b67050 [0x2587ec8]>{contents = "callbackId"} = <CFString 0x9b6e100 [0x2587ec8]>{contents = "cb_1_1407401617323"}
}

The application continues to run without crash.

When we use NSLog, we will see the following message in the log:

WVJB Warning: No handler for message from JS: {
    callbackId = "cb_1_1407402032394";
    data = "Hello from JS button";
}

The application continues to run without crash.

When we use current implementation (without checking of the handler), we do not see anything in the log, only EXC_BAD_ACCESS.

The application has crashed.

Thanks again!

Sincerely,
Ruslan

[ruslanskorb]

In addition to the previous comment, I would like to tell you about my situation.

[1] I initialize the bridge in my app using of the method bridgeForWebView: webViewDelegate: handler: where I pass the parameter handler as nil, because I was only interested in certain messages from JS.
[2] Next, I register the desired handlers using of the method registerHandler: handler:.
[3] After that, I send my app to testers.
[4] At this time, for experimental purposes, my web developer add the button on the site that sends a message to the Obj-C without handlerName.
[5] Testers see this button and click on it.
[6] As a result, the app has crashed and I have confused.

If you want to leave the handler as the required parameter in the method bridgeForWebView: webViewDelegate: handler:, then I would suggest to indicate it more clearly. For example, you can add NSParameterAssert in initializer.

Thanks!

[marcuswestin]

I think you're making a very good argument for logging the error and not crashing. I still wonder this though:

Don't you think a tester will be more confused if a button does nothing ("Did I click it? Did I miss the button? Does it do something that I'm not noticing? Did I just submit that post, or didn't I?") and that bug reports will be less clear ("App crashes when I tap the submit button" vs "Submit button doesn't work")?

[marcuswestin]

btw, I really appreciate that you're taking your time to structure a logical argument about this. It's a nice contrast to most feature requests :)

[yas375]

@marcuswestin I'd like to confirm that we also pass in empty handler which does nothing when we initialise the bridge and then we set up needed message and handlers using registerHandler:handler:. So having to pass an empty handler, but not nil makes me to worry about a little.

To have a control of the code we ship I think it makes sense to pass in at least empty handler to the initialiser of the bridge to avoid such crashes. Because probably the html/js part is not directly controlled by us and it makes me scary to make assumptions that HTML returned by a server sends only valid messages. Like in the case @ruslanskorb has described a web developer can add some new button. Or maybe you are adding a new button, but old clients don't have handlers for its message and you don't have versioning infrastructure right now. Maybe in this case it would be better for you to get reports from users of the old app version like "I've noticed a new button, but looks like it doesn't work" instead of "your app creates when I hit the new button". User don't like crashes ;) It's probably fine to crash during debugging (NSAssert for the rescue), but I would prefer not to crash in production and better to receive feedback that something doesn't work instead of crashes. That's my IMO ;)

[ruslanskorb]

@marcuswestin I think, that the ability to continue working without crashes is more important :) But, this is my opinion and the opinion of our testers :) You can try to imagine a situation when the button accidentally gets into production and a real user (not the tester) has pressed it. This situation will be very uncomfortable. One "bug report" of the real user will cause you more harm than a lot of bug reports of testers ⚡ 🔥

@yas375 👍 Users don't like crashes, especially in the production! :)

[marcuswestin]

Alright. You win :) Thanks for sticking with the argument. Am making one more change before tagging new version.

[ruslanskorb]

@marcuswestin Thank you for merging! 😃 👍