Basic presentation of form load failures; prepare release: @getodk/web-forms 0.5.0
#237
Conversation
Where a “form load failure” is any Promise rejection produced by the engine’s `initializeForm` entrypoint. This is intentionally broad, and what I think is a relatively minimal first pass that might be useful to users (and to us). “Relatively” minimal because I took the opportunity to lay a bit of groundwork for a few things either cued for near term work, or ripe for near term improvement. Notable among those: - Establish a more intentional approach to use of test fixtures in the `web-forms` package. This includes a dedicated fixtures directory (alongside those already used by `scenario`), and a narrower “test helper” (ugh) to access those. That access point also includes some additional breadcrumbs for ways we might improve fixture access in the future. - `FormLoadFailure` begins laying a bit of groundwork for the incoming, much more comprehensive overhaul of error messaging. - Actually tests a few different form load failure conditions. I believe these will be useful for catching potential regressions in that overhaul, as well in other future work around those specific failure modes. UI/UX/presentation design note: I had a brief discussion with @alyblenkin about this being a minimal first step. We agreed that this seems like a reasonable first pass, and agreed there’s more to do in near-future iterations. Things which are currently lacking from a UX perspective, blocked by work with a larger scope: - Better messaging of specific errors. We’ll be cataloguing known cases as part of #202, which will give us a much clearer picture of how to address this. - Any kind of direct prompt for user action. Deferred for the same reasons. - Ability to close the dialog or go back. Deferred because this would expand scope to navigation more generally, which we should look at more holistically.
It would make a lot more sense to just include all `.ts` source files, but that causes issues with the handful of `.ts` entrypoint-ish modules which happen to also import `.vue` modules. That’s a fundamental problem with the Vue template language as a pseudo-module format, just like how it breaks down with ESLint (and probably will for many other tools!).
For some reason, testing in Firefox fails unpredictably when asserting text content of an element where word breaks occur at element boundaries! I had hoped not to use a more specific selector here, but it’s gonna have to do...
|
| * {@link unknownCauseDetail}, in hopes it will provide further clarification | ||
| * of unknown failure conditions. | ||
| */ | ||
| export class InitializeFormFailure extends Error { |
There was a problem hiding this comment.
perhaps a better name for this class could be FormInitializationFailure. Or even FormInitializationError, making it clear to passers-by that it's specialization of Error.
There was a problem hiding this comment.
Normally I'd agree! The naming here is trying to accomplish—or start to accomplish—a few things which aren't typical:
- Make it explicit that the class is directly coupled to the
initializeFormfunction - Make it clear that the condition is unambiguously a failure of a call to that function
- Set this case up for incorporation into very near-term work on External secondary instances: Error conditions #202
In near term work (and application of the approach generally), we'll have at least some notion of:
-
Errors/error-like conditions which correspond directly to specific operations, computational concepts, spec domain model concepts, etc. To the extent they're coupled, naming them as such seems helpful.
-
Levels of severity which may be objective (e.g. this case, "
initializeFormfailed") but may have some room for subjective interpretation (such as failure to access some form attachment, which could be terminal for some use cases but perfectly fine for others). -
In support of that, errors/error-like conditions produced together with requested values.
-
Type-level and API documentation incorporating those types, in a way that conventional
throwsemantics don't allow.
All of that aside, I'm perfectly fine with preferring an Error suffix here as the specific terminology to convey "this operation unambiguously failed". This is definitely a case that most overlaps with that idiom. But I do want to make sure all of this context around the broader approach to error production doesn't get lost in naming this one class.
Note to myself: whatever we decide on the name, the module name is already inconsistent with the class. So one or both should change.
- For now, we’ll go with the name suggested in review. As discussed there, we have broader goals for error reporting, but we can address those in a non-bandaid change (i.e. supporting #202) - Aligns the module and class name
eyelidlessness
changed the title
@getodk/web-forms 0.5.0
I have verified this PR works in these browsers (latest versions):
What else has been done to verify that this works as intended?
A few tests. I think we'll want to expand testing in this area significantly, probably at different levels of the stack.
Why is this the best possible solution? Were any other approaches considered?
It's approximately the quickest solution, which was the priority for now. There are known areas for improvement, discussed under What's changed below.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
If users previously depended on getting no information whatsoever when a form fails to load, this will break that workflow.
Do we need any specific form for testing your changes? If so, please attach one.
Added a few fixtures, each demonstrating a known failure case.
What's changed
Here, a “form load failure” is any
Promiserejection produced by the engine’sinitializeFormentrypoint. This is intentionally broad, and what I think is a relatively minimal first pass that might be useful to users (and to us).“Relatively” minimal because I took the opportunity to lay a bit of groundwork for a few things either cued for near term work, or ripe for near term improvement.
Notable among those:
Establish a more intentional approach to use of test fixtures in the
web-formspackage. This includes a dedicated fixtures directory (alongside those already used byscenario), and a narrower “test helper” (ugh) to access those. That access point also includes some additional breadcrumbs for ways we might improve fixture access in the future.FormLoadFailurebegins laying a bit of groundwork for the incoming, much more comprehensive overhaul of error messaging.Actually tests a few different form load failure conditions. I believe these will be useful for catching potential regressions in that overhaul, as well in other future work around those specific failure modes.
UI/UX/presentation design note
I had a brief discussion with @alyblenkin about this being a minimal first step. We agreed that this seems like a reasonable first pass, and agreed there’s more to do in near-future iterations. Some of the things which are currently lacking from a UX perspective, blocked by work with a larger scope: