framework icon indicating copy to clipboard operation
framework copied to clipboard

Misleading error message if snippet returns null

Open aweinert opened this issue 8 years ago • 4 comments

Summary

When a snippet returns null, the error message shown in the browser states that the corresponding method could not be found. This is somewhat misleading. I think it would be more informative if it were changed to something like snippet.method returned null. I have uploaded a small project showing this error at https://github.com/aweinert/liftweb-null-snippet.

What I did

$ git clone https://github.com/aweinert/liftweb-null-snippet
<...snip...>
$ cd liftweb-null-snippet
$ ./sbt
> container:start
<...snip...>

Then, open a browser and navigate to 127.0.0.1:8080

What happened

An error is displayed, as shown here: screenshot

What I expected to happen

An error should indeed be displayed, but I think the error message is somewhat misleading. While it is probably technically correct, it should point to a null-pointer-issue rather than to a method resolution one.

Previous discussion

I have also posted this issue to the mailing list.

aweinert avatar May 19 '16 07:05 aweinert

I think we'll want to add a new error state to LiftRules.SnippetFailures and guard for null in the places where we invoke the result of the function blindly:

https://github.com/lift/framework/blob/79d036703eedf32a27485574ded1bf2805421ead/web/webkit/src/main/scala/net/liftweb/http/LiftSession.scala#L1689 https://github.com/lift/framework/blob/79d036703eedf32a27485574ded1bf2805421ead/web/webkit/src/main/scala/net/liftweb/http/LiftSession.scala#L1678 https://github.com/lift/framework/blob/79d036703eedf32a27485574ded1bf2805421ead/web/webkit/src/main/scala/net/liftweb/http/LiftSession.scala#L1667 https://github.com/lift/framework/blob/79d036703eedf32a27485574ded1bf2805421ead/web/webkit/src/main/scala/net/liftweb/http/LiftSession.scala#L1657

Though the truth is a broader refactoring of snippet dispatch is also long overdue.

Shadowfiend avatar Dec 12 '16 02:12 Shadowfiend

I'm going to schedule this for 3.2 with an eye towards refactoring snippet dispatch a bit. We've done some messing with this at work anyway, and I'd like to give it a proper cleanup and expose the important bits for framework user consumption, and fix some of these edge cases while I'm in there.

Shadowfiend avatar Apr 22 '17 19:04 Shadowfiend

Also I'd like to accidentally close this issue <_<

Shadowfiend avatar Apr 22 '17 19:04 Shadowfiend

Started looking at this a bit, btw. Lot of code to move around, but still hoping to get it done for 3.2.

Shadowfiend avatar Jul 17 '17 20:07 Shadowfiend