Never let stray errors in the console

When doing web development I fix all the errors that show up in the console. Often they don’t seem to impede the actual use of the software, but nevertheless I think it’s a good practice to fix them all. This following example should illustrate why.

I was doing some maintenance on an Angular project at work. I noticed that there were a couple of errors showing up in the console, but the web site still worked great, even with the errors.

The reason I was working on this project was because of a bug that was reported by QA. There was a display error that was happening and it was related to animations that were used in the Angular transitions, these are used when switching from one component to another. The error manifested itself with Android phones and iPhones. This bug probably slipped by because everything was working fine on desktop browsers.

After investigating, I saw that there was an error in the console about calling a property on undefined. On a view, this can be fixed using the ?. operator. The view used the ?. operator elsewhere but there was one place it was missing. I was sure this couldn’t be what was messing up the display of the animated transition.

I searched and searched but couldn’t find the problem.

So I decided that while I was there, I might as well fix this error that showed up in the console. I changed the . operator for ?. and lo and behold this also fixed the display problem.

By correcting it, it also corrected the other problem, which was probably caused by having an error during the Angular rendering.

This example demonstrates that you should never let console errors live, even if they don’t seem to have any negative effect on the web site.

Tales from the trenches: ASP.Net ViewState bug

This post is part of a series of actual problems or bad practices I encountered working on existing C# code bases.

The names of all identifiers, the exact functionalities of the programs and the code samples have all been altered for confidentiality reasons.

While looking into a slow ASP.Net page I noticed that the page was making a lot more service calls than it should have. Visually I could see superfluous calls that should have been merged together.

I profiled the page with the Performance Tools in Visual Studio and also put breakpoints on all service calls and ran it in Debug mode.

One thing that stood out was one property that was supposed to be using ViewState to prevent calls to the service layer but actually wasn’t.

It went something like this :

public int Somevalue 
{
    get 
    {
        if (ViewState["SomeValue"] == null)
        {
            var foo =
            ValueService.GetSomeValue(Convert.ToInt32
                (Request["SomeValue"]));

            ViewState["SomeValue"] = foo;
        }
        
        return (int)ViewState["SomeValue"];
    }
    set 
    {
        ViewState["SomeValue"] = value;
    }
}

// ...

protected override void OnInit(EventArgs e)
{
    int x = SomeValue;	// contrived example
}

I noticed that ValueService.GetSomeValue was getting called on PostBacks. Looking at where it was used, I saw it was being called in the OnInit method. It just so happens that during the OnInit phase of the ASP.Net page life cycle, the ViewState hasn’t be loaded with it’s values. This effectively means that SomeValue will always be null in the OnInit phase and it will have to make a service call every time.

The solution in this case was simply to move the call to a later stage of the page life cycle, in this case the PageLoad event.

The take away from this is to be mindful of the ASP.Net page life cycle events. Also putting breakpoints on service calls while loading the page and submitting the form is a quick way to see if something is amiss with your service calls.

Tales from the trenches: ASP.Net static variable bug

This post is part of a series of actual problems or bad practices I encountered working on existing C# code bases.

The names of all identifiers, the exact functionalities of the programs and the code samples have all been altered for confidentiality reasons.

While working on an ASP.Net control bug, I noticed this more important problem which hadn’t been reported.

The reported bug was about a control that would sometimes not repopulate itself. It was a pretty simple case.

if (ConditionA && lastValue != currentValue)
{
    lastValue == currentValue;
    // more code
}

What happened is that when ConditionA failed, lastValue wasn’t updated. The fix was simply to move the lastValue assignment statement outside of the conditional.

The bigger problem I noticed was when I decided to look at how lastValue was implemented.

It turned out it was implemented as a static member variable. This being an ASP.Net page, a static variable is shared between all instances of the class in the AppDomain.

What this effectively means is that every user of the site shares this same variable.

Suppose user A fills lastValue with “test”, then user B fills currentValue with “test”, the server-side code detects that user B has already used this value as his value (since it’s currently in lastvalue), even if he hasn’t entered anything before. This could lead to some inconsistent behaviour.

The lesson here is two-fold:

1- Do not to cache information with a static variable in an ASP.Net site unless that information is not subject to change from user to user.

2- When working on existing code, make sure you understand correctly how it is implemented. In this case I could have glossed over the implementation of lastValue, which wasn’t declared near the method I was working on and missed this important bug.