I'm often opening, from Excel, all Word documents inside a folder.
And sometimes during the treatments, Word crash without throwing an error.
So I've added a bit of error handling to restart Word and reopen the doc I was working on.
I'm just wondering if I'm doing it properly, or if I should go further like :
- Test the error number
- If it's 462 : The remote server machine does not exist or is unavailable, go to RestartWord
- If it's something else, show the error message
It may also not be only Error 462, but I don't have any other examples for this.
My code :
Private Sub Update_Word_DocMs_In_Folder(ByVal FolderToScan As String, _
ByVal PathTxtWithInfo As String)
Dim oW As Object, NewFile As String
'''Boolean to detect if it was the initial launch
Dim NormalWordLaunch As Boolean
NormalWordLaunch = True
Set oW = OpenOrReopen("Word.Application", NormalWordLaunch)
On Error GoTo RestartWord
NewFile = Dir(FolderToScan & "*.docm")
Do While NewFile <> vbNullString
'''ErrHdl: Jump back here after re-creating a Word's instance
ReOpenWordFile:
Call OpenWordDocAndApplyTreatment(oW, FolderToScan & NewFile, PathTxtWithInfo)
NewFile = Dir()
Loop
oW.Quit
On Error GoTo 0
Set oW = Nothing
Set oDoc = Nothing
Exit Sub
RestartWord:
If Err.Number <> 462 Then
MsgBox Err.Number & vbCrLf & Err.Description, vbCritical + vbOKOnly, "Error not handled"
DoEvents
Resume
Else
Set oW = OpenOrReopen("Word.Application", NormalWordLaunch)
'''ErrHdl: If this isn't the initial launch go back to the loop
If Not NormalWordLaunch Then Resume ReOpenWordFile
End If
End Sub
Function to handle creating an app instance :
Function OpenOrReopen(ByVal ClassName As String, NormalLaunch As Boolean) As Object
Set OpenOrReopen = Nothing
On Error Resume Next
'''Check if there is already an instance
Set OpenOrReopen = VBA.GetObject(, ClassName)
On Error GoTo 0
If OpenOrReopen Is Nothing Then
'''If there isn't any instance, create one
Set OpenOrReopen = VBA.CreateObject(ClassName)
Else
'''If detected instance is visible, create a new one
If OpenOrReopen.Visible Then Set OpenOrReopen = VBA.CreateObject(ClassName)
End If
'''Set instance to be hidden
OpenOrReopen.Visible = False
OpenOrReopen.Options.UpdateLinksAtOpen = False
'''For ErrHdl: Change boolean the 1st time
If NormalLaunch Then NormalLaunch = False
End Function
Code for treatment :
Sub OpenWordDocAndApplyTreatment(WordApp As Object, _
ByVal FileFullName As String, _
ByVal PathTxtWithInfo As String)
Dim oDoc As Object
Set oDoc = WordApp.Documents.Open(FileFullName)
With oDoc
Do While .ReadOnly
DoEvents
Loop
'''Some treatment to update doc's properties
WordApp.Run "Properties_Updating.Properties_Update", PathTxtWithInfo
DoEvents
.Save
.Close
End With 'oDoc
End Sub
1 Answer 1
The name Update_Word_DocMs_In_Folder
isn't following the PascalCase
convention that's otherwise observed by your code; underscores should generally be avoided in procedure names in VB6/VBA (especially for Public
members), because underscores have a very specific meaning in identifier names:
[Interface]_[Member]
[EventSource]_[EventName]
So Properties_Update
is a problem, or can become one.
I find the indentation hard to follow, it doesn't seem consistent: some statements aren't indented, others are, but there's no apparent rhyme or reason to it.
The Call
statement is obsolete and its use isn't consistent either; I'd remove it.
Call OpenWordDocAndApplyTreatment(oW, FolderToScan & NewFile, PathTxtWithInfo)
Becomes:
OpenWordDocAndApplyTreatment oW, FolderToScan & NewFile, PathTxtWithInfo
The RestartWord
label is misleading: it's just an error handler subroutine, it doesn't necessarily "restart Word".
oDoc
and oW
have that useless and annoying Systems Hungarian prefix ("o" for "object", right?) and the identifiers are rather poorly named. I would've had wordApp
instead of oW
(funny just noticed that's quite exactly what another procedure is calling it), and oDoc
is declared and assigned to Nothing
, but it's never assigned an actual object reference, never passed ByRef
to anything, and never referenced either: it's not used, and should be removed.
Not sure why CreateObject
is being fully-qualified with the VBA
standard library identifier - seems rather arbitrary. Why isn't MsgBox
fully-qualified then?
I find this a bit dangerous:
MsgBox Err.Number & vbCrLf & Err.Description, vbCritical + vbOKOnly, "Error not handled" DoEvents Resume
If the error is unknown and unhandled, IMO the right thing to do is to gracefully halt execution, not enter a potentially infinite "try that again in an unknown error state" loop.
If Not NormalWordLaunch Then Resume ReOpenWordFile
I don't like this jumping into a loop body. If OpenWordDocAndApplyTreatment
is what's throwing error 462, then OpenWordDocAndApplyTreatment
is what should be handling error 462 - I'd make the procedure a Function
that returns False
when that error is thrown and the Word instance needs to be reset.
Honestly, that procedure-that-should-be-a-function wants its own error handling: it can blow up given an invalid path/filename, or Documents.Open
fails for whatever reason, or there's no Properties_Updating
module, or there's no Properties_Update
macro, or .Save
fails - so many things can go wrong in that scope, and all of it is just bubbling back up to the calling code's loop body - that's why you have all this jumping-around.
Handle these errors in OpenWordDocAndApplyTreatment
, and the loop body and its error-handling logic can get much more straightforward.
OpenOrReopen
has its side-effects backwards IMO. I would have returned a Boolean
and altered an Optional ByRef outWordApp As Object = Nothing
parameter. The ClassName
parameter is misleading too: that's a registered ProgId
, not a ClassName
. Not sure why a ProgId would be needed though - it looks like you could use it to get any registered object, but then there's this:
OpenOrReopen.Visible = False OpenOrReopen.Options.UpdateLinksAtOpen = False
Which, aside from treating the return value as if it were a declared local variable (a bad practice IMO), assumes things it shouldn't be assuming about the object's members - if it's meant to only work with a Word.Application
, then don't take a ProgId parameter, hard-code it in (or make it a Const
somewhere, but don't pretend it's a parameter).
SELECT CASE
rather thanIF
on your error handler - easier to group any errors then. \$\endgroup\$