Report wizard start failures instead of failing silently (bz-80429) - #495
Open
leginee wants to merge 2 commits into
Open
Report wizard start failures instead of failing silently (bz-80429)#495leginee wants to merge 2 commits into
leginee wants to merge 2 commits into
Conversation
Issue 80338 comment 21 reports that the table wizard dialog simply does not
appear, with no error message of any kind, and correctly guesses that the
underlying error "is caught and ignored somewhere (libdbaxml680li.so?)". It is
in dbaxml, and there are two independent silent paths.
dbaccess/source/filter/xml/dbloader2.cxx, started automatically after a new
database document is created:
if ( m_aContext.createComponentWithArguments(
"com.sun.star.wizards.table.CallTableWizard", aWizArgs, xTableWizard ) )
xTableWizard->trigger( "start" );
}
catch(const Exception&)
{
OSL_ENSURE(sal_False, "caught an exception while starting the table wizard!");
}
createComponentWithArguments() returns false rather than throwing when the
service is not registered, and there is no else branch -- so nothing happens and
nothing is said. When creation does throw, which is the case in the original
report (the wizard is a Java component and JNI_proxy.dispatch_call cannot be
linked), the exception is discarded by OSL_ENSURE, which is debug-only and
therefore does nothing in a product build.
dbaccess/source/ui/misc/linkeddocuments.cxx, behind "Use Wizard to Create
Table/Query/Form/Report", has the same second problem: UNO_QUERY_THROW does turn
a missing service into an exception, but the catch discards it with
DBG_UNHANDLED_EXCEPTION, also debug-only.
Both now show the error. Notes on the shape of the fix:
- OnStartTableWizard must not do modal UI inline. It runs from a VCL user
event, and m_xMySelf holds the loader alive across that event; a message box
pumps a nested event loop, during which the document and frame creation the
handler was posted from keeps running and can drop the last reference. So
the error is collected first, all member state is settled -- with a local
keep-alive making it safe to release the self-reference early -- and only
then reported, so nothing touches the object after the nested loop.
linkeddocuments.cxx needs none of this: it is a direct UI action with a real
parent window and no self-reference.
- The text displayed is the underlying error's own message, so this adds no
new localizable string and needs no translation work. A properly worded and
translated wrapper would be nicer and is a reasonable follow-up; it is
deliberately not done here to keep the change small.
- ErrorBox rather than the InteractionHandler or dbaui::showError, both of
which would have stayed silent for this class of error. uui's
handleRequest_impl matches specific request types and returns false for
anything else (after another debug-only OSL_ENSURE), and showError is
documented as showing nothing when the SQLExceptionInfo is not valid, which
a plain RuntimeException is not. ErrorBox is already reachable from both
libraries -- each file already includes vcl/msgbox.hxx.
- The exception must not be rethrown. An escaping exception reaches
Desktop::Main's catch-all, which calls FatalError() and then _exit() --
turning a silent no-op into killing the office with unsaved work in other
windows.
- The reporting itself is wrapped in a catch-all so that reporting an error
can never replace it with a different one, and it takes the solar mutex
because in the throwing path the caller's guard has already been unwound.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…years
NativeLibraries.load() preloads native libraries on Windows before
System.loadLibrary("hsqldb"). Two of the names are dead:
msvcr71 the Visual Studio 2003 C runtime. AOO has not built against it
for a very long time, and the CRT is in any case resolved through
the side-by-side assembly named in each library's manifest, not by
preloading. Removed rather than renamed.
dbtoolsmi "dbtools" with the DLLPOSTFIX of a much older Windows platform
set appended. DLLPOSTFIX is empty in every wntmsci*.mk, so the
library has been plain dbtools.dll for as long as those files have
looked the way they do. Corrected to "dbtools".
Both therefore always throw UnsatisfiedLinkError, which loadLibrary() below
swallows when the classloader lookup also comes up empty -- so this has been
completely invisible, which is presumably why it survived. The preloads are
belt and braces anyway: in the office hsqldb.dll is already loaded as a UNO
component with its imports resolved, so nothing depended on them succeeding.
Not a behaviour change beyond removing two failing LoadLibrary calls per
connection. Left alone deliberately: uwinapi and sal3, which do exist and do
load. (LibreOffice dropped uwinapi from this same list, but only because they
removed that library outright; both stale names are still present there too.)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
leginee
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
In Base, choose Tables → "Use Wizard to Create Table…". If the wizard cannot start, Apache OpenOffice today does nothing whatsoever — no dialog, no message, no log entry. The menu item simply appears not to work, and there is no way for a user or a support volunteer to find out why.
This was reported 18 years ago, in issue 80429 a spin off of issue 80338, comment 21:
"It is bad that the wizard dialog silently does not appear without any error message."
That reporter also guessed where the error was being thrown away — "caught and ignored somewhere (libdbaxml680li.so?)". They were right, and this PR fixes it at that exact spot.
Why it was silent
Two independent places, both in dbaccess:
dbloader2.cxx (the wizard that starts automatically after you create a new database). It asked for the wizard component and checked whether it got one — but if the answer was "no", there was no else branch at all. Nothing happened, and nothing was reported. If the request instead failed with an error, that error was passed to OSL_ENSURE, a debugging aid that is compiled out of release builds — so again, nothing.
linkeddocuments.cxx (the "Use Wizard to Create…" menu entries for tables, queries, forms and reports). Here the failure was turned into a proper error, but it was then discarded by DBG_UNHANDLED_EXCEPTION, which is also debug-only.
In short: in the builds users actually run, both paths threw the explanation away.
What changes
Both now show an error box naming the component that could not be started, for example:
com.sun.star.wizards.table.CallTableWizard:
unsatisfied query for interface of type com.sun.star.task.XJobExecutor!
Not beautiful, but it turns "the button does nothing" into something diagnosable.
Notes for reviewers
No new translatable strings. The text is the underlying error's own message, prefixed with the service name, so there is no translation work and nothing to re-import.
ErrorBox, not InteractionHandler or dbaui::showError. Both were tried first and both would have stayed silent: uui's handleRequest_impl matches specific request types and returns false for anything else, and showError is documented to display nothing when the SQLExceptionInfo is not valid — which a plain RuntimeException is not. ErrorBox is already reachable from both libraries.
The exception is deliberately not rethrown. OnStartTableWizard runs from a VCL user event; an escaping exception reaches Desktop::Main's catch-all, which calls FatalError() and then _exit() — turning a silent no-op into killing the office with unsaved work in other windows.
No modal UI inside the user-event handler. m_xMySelf keeps the loader alive across that event, and a message box pumps a nested event loop during which the document/frame creation that posted the handler can drop the last reference. The error is therefore collected first, member state settled (with a local keep-alive), and only then reported. This was found the hard way — an earlier version of this patch crashed on exactly that path.
Second commit, unrelated
sdbc_hsqldb: stop preloading two libraries that have not existed for years removes two dead System.loadLibrary calls on Windows:
msvcr71 — the Visual Studio 2003 C runtime. Not what AOO builds against, and the CRT is resolved through the side-by-side assembly in each library's manifest anyway.
dbtoolsmi — dbtools with the DLL suffix of a much older Windows platform set. DLLPOSTFIX is empty in every wntmsci*.mk, so the library has been plain dbtools.dll for as long as those files have looked the way they do.
Both always failed, and the failure is swallowed, so this has been invisible rather than harmful. The same two names are still present in LibreOffice.
The two commits are independent — happy to split them into separate PRs if preferred.
Testing
Built and exercised on Windows x64. The table wizard entry point now shows the dialog above where it previously did nothing; creating a database, adding a table, saving and reopening all work, with a clean process exit.
🤖 I generated this PR with Claude Code