1

I am writing some code that opens up a number of files via a url. This all works fine, however after a while the server I am pulling this data from blocks me, which is throwing up an error message.

What I have tried to do is create an error handler that resets the error and then continues from the top after waiting 5 seconds. I have tried two things

  1. On error resume next, to skip that line. This doesn't seem to do anything as the code still times out.

  2. Go to error handler, wait 5 seconds, reset the error and then continue where the code already was.

Any ideas what I am doing wrong. example file paths below;

https://query1.finance.yahoo.com/v7/finance/download/GBPUSD=X?period1=946684800&period2=9999999999&interval=1d&events=history

https://query1.finance.yahoo.com/v7/finance/download/GBPCNY=X?period1=946684800&period2=9999999999&interval=1d&events=history

https://query1.finance.yahoo.com/v7/finance/download/^NZ50?period1=946684800&period2=9999999999&interval=1d&events=histor

Sub TESTING()

Call START

Dim i As Integer

    Workbooks("SHARE PRICE CREATOR.xlsb").Sheets("links").Activate

For i = 2 To Application.WorksheetFunction.CountA(Range("E:E"))

    xtable = Cells(i, 5)
    xURL = Cells(i, 4).Value
    
CONTINUE:
    
    On Error GoTo Errhandle
    Workbooks.Open xURL, FORMAT:=6, DELIMITER:=","
    Workbooks("SHARE PRICE CREATOR.xlsb").Sheets("links").Activate
    Cells(i, 6) = "OK"
    
Next

Errhandle:
    On Error Resume Next
        If Err.Number > 0 Then
            Cells(i, 6) = Err.Number
        End If
    On Error GoTo 0
    Application.Wait (Now + TimeValue("0:00:5"))
    
    GoTo CONTINUE

Call ENDING
    
End Sub

Thanks

Scott

Scottyp
  • 111
  • 1
  • 10
  • Side note: I don't think the On Error Resume Next serves any purpose in your ErrHandle. Put _Workbooks("SHARE PRICE CREATOR.xlsb").Sheets("links")_ into a variable and qualify your range calls with that. Avoid implicit Activesheet references. – QHarr Jun 21 '20 at 14:29
  • Use Err.Clear to clear error – QHarr Jun 21 '20 at 14:35
  • 1
    You will need an Exit Sub for successful completion if all tasks before running into your error handler. Also, you need a exit strategy to avoid potential for infinite loop. I personally would go with a max retries strategy before moving onto next url and also have a wait every x number of requests to be a good netizen. – QHarr Jun 21 '20 at 14:45
  • The wait every 20 cycles was option 3, I have no idea how to do that though other then duplicating the code with different ranges for i – Scottyp Jun 21 '20 at 15:03

1 Answers1

1

Some pointers:

  1. I don't think the On Error Resume Next serves any purpose in your ErrHandle
  2. Put Workbooks("SHARE PRICE CREATOR.xlsb").Sheets("links") into a variable and qualify your range calls with that
  3. Avoid implicit Activesheet references
  4. Use Err.Clear to clear error
  5. You will need an Exit Sub for successful completion of all tasks before running into your error handler
  6. You need an exit strategy to avoid potential for infinite loop. I personally would go with a max retries strategy before moving onto next url and also have a wait every x number of requests to be a good netizen
  7. Generally avoid the spaghetti code effect of GoTo
  8. Declare all your variables with their type. Remove if not used. Use Option Explicit to enforce

Generally:

I don't like GoTos as makes code hard to read and debug. See a possible re-write, with further comments, below:


TODO:

Refactor out code to be less nested with use of helper functions/subs i.e. be more modular.


Code:

Option Explicit 'Use Option Explicit

Public Sub RetrieveYahooData()

    Const MAX_RETRIES As Long = 3
    Dim i As Long, ws As Worksheet, lastRow As Long 'use Long
    Dim wbMain As Workbook, wb As Workbook, xUrl As String  'declare xUrl
    Dim xtable As String 'temp assignment.
     
    Start 'what subs are these?
    
    Set wbMain = Workbooks("SHARE PRICE CREATOR.xlsb") ''Put in a variable. This assumes is open.
    Set ws = wbMain.Worksheets("links")
    
    lastRow = ws.Cells(ws.Rows.Count, "D").End(xlUp).Row 'You want to count from row 2 I think
    
    If lastRow >= 2 Then
    
        For i = 2 To lastRow
            
            If i Mod 100 = 0 Then Application.Wait Now + TimeSerial(0, 0, 5) 'every n e.g. 100 requests have a pause
            
            numberOfTries = 0
            
            With ws
            
                xtable = .Cells(i, 5).Value      '?What is xTable and its datatype? _
                                                 Declare it and use Option Explicit at top of code. _
                                                 Also, where will it be used?
                xUrl = .Cells(i, 4).Value
                
                If xUrl <> vbNullString Then
                    
                    Do
                    
                        DoEvents
                        
                        On Error Resume Next
                    
                        Set wb = Workbooks.Open(xUrl, Format:=6, DELIMITER:=",") 'add other tests for valid url?
                        
                        On Error GoTo 0
                        
                        If Not wb Is Nothing Then 'remember to save and exit do
                            wb.SaveAs wbMain.Path & "\" & wb.Name, AccessMode:=xlExclusive, ConflictResolution:=Excel.XlSaveConflictResolution.xlLocalSessionChanges 'Credit to @Sorceri https://stackoverflow.com/a/14634781/6241235
                            wb.Close True
                            Exit Do
                        Else
                            Application.Wait Now + TimeSerial(0, 0, 5)
                        End If
                     
                    Loop While numberOfTries < MAX_RETRIES
                    
                End If
    
            End With
            
            ws.Cells(i, 6) = IIf(wb Is Nothing, "FAIL", "OK")
          
            Set wb = Nothing
        Next
    End If
          
    ENDING

End Sub
QHarr
  • 83,427
  • 12
  • 54
  • 101