bradyt / taskw-dart

Taskwarrior-inspired mobile todo app
82 stars 8 forks source link

Creating a task with an empty description explodes rather abruptly #18

Open danielquinn opened 2 years ago

danielquinn commented 2 years ago

I accidentally created a task with no description. I just tapped the plus icon:

plus

and then tapped the "enter" icon:

enter

And this happened:

FormatException: Empty description will provoke a server error. (at character 1)

^
#0      validateTaskDescription (package:taskw/src/validate.dart:3)
#1      new Task._ (package:taskj/src/json/task.dart:40)
#2      new _$Task._ (package:taskj/src/json/task.g.dart:362)
#3      TaskBuilder.build (package:taskj/src/json/task.g.dart:624)
#4      new _$Task (package:taskj/src/json/task.g.dart:336)
#5      taskParser (package:taskw/src/task_parser.dart:76)
#6      _AddTaskBottomSheetState.submit (package:task/src/widgets/add_task_bottom_sheet.dart:34)
#7      _InkResponseState._handleTap (package:flutter/src/material/ink_well.dart:989)
#8      GestureRecognizer.invokeCallback (package:flutter/src/gestures/recognizer.dart:198)
#9      TapGestureRecognizer.handleTapUp (package:flutter/src/gestures/tap.dart:608)
#10     BaseTapGestureRecognizer._checkUp (package:flutter/src/gestures/tap.dart:296)
#11     BaseTapGestureRecognizer.acceptGesture (package:flutter/src/gestures/tap.dart:267)
#12     GestureArenaManager.sweep (package:flutter/src/gestures/arena.dart:157)
#13     GestureBinding.handleEvent (package:flutter/src/gestures/binding.dart:443)
#14     GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:419)
#15     RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:322)
#16     GestureBinding._handlePointerEventImmediately (package:flutter/src/gestures/binding.dart:374)
#17     GestureBinding.handlePointerEvent (package:flutter/src/gestures/binding.dart:338)
#18     GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:296)
#19     GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:279)
#20     _rootRunUnary (dart:async/zone.dart:1444)
#21     _CustomZone.runUnary (dart:async/zone.dart:1335)
#22     _CustomZone.runUnaryGuarded (dart:async/zone.dart:1244)
#23     _invoke1 (dart:ui/hooks.dart:169)
#24     PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:293)
#25     _dispatchPointerDataPacket (dart:ui/hooks.dart:88)

While it's obviously ok that the app wouldn't want to create an "empty" task, a friendlier error message would probably be better.

bradyt commented 2 years ago

I haven't been able to think of a simple solution. Here's some of what I've considered:

None of these seem like a good fit to prioritize currently.

Just to be clear, I'm not proposing to close the issue. I appreciate the dialogue, and will keep this in mind. Something may come to me, or you, or someone reading this. Or this may influence some future restructuring, or priorities may shift.

danielquinn commented 2 years ago

I'm afraid I'm not much help in Dartland, but in Pythonland, I'd adopt a pattern of catching the exception and just rendering a friendlier message:

try:
    store_task(description)
except Exception as e:
    render_pretty_dialogue(str(e))  # str(e) gets you the message from the exception

The idea being that the exception traceback isn't useful to you as you know what happened and that this sort of thing is understood to be not ok, so just show a friendly pop-up message instead.

Alternatively, you can just test the description before trying to do anything else (Python again, sorry):

error = validate_description(description)  # Does all the validation
if error:
    render_pretty_dialogue(error)
    return
store_task(description)

At the end of the day it's your call. If you wouldn't classify this as something to worry about it, by all means close it as wontfix. I'm already very happy with the app and thought that I was helping by suggesting some areas that could improve the look/feel is all :-)

bradyt commented 2 years ago

Your examples are approximately what's going on. It's a try catch block in the UI code, the Task constructor calls validation methods that can throw exceptions, the UI code catches the exception, and has access to both the exception and the trace. I decided to share the trace, as I felt that made it easier for me the developer. (Off-topic but, I wasn't immediately able to lift the call to validate out of the UI code for the tag editing UI.)

I can think of a few potential issues if I try to simply drop the trace for this one scenario. Overall it's just not something I want to rush into right now, given the state of the project, and other improvements I am considering to prioritize. I don't know that I'll never improve this. Maybe a month from now, a simple fix will come to mind, or I'll spontaneously decide it's worth it. Or maybe a year or five years, I'll decide the project is in a state to work on this.

I am glad you are happy with the app still. I appreciate your patience with the lack of polish here.