Page MenuHomePhabricator

Be more specific about falsy values in IP Info api [1H]
Closed, ResolvedPublicSpike

Description

This ticket describes what I think is an architecture bug causing downstream symptoms. Ideally we come to a consensus about how we want to treat these falsy values and possibly split some working tickets off from that.

IP Info prefers to treat all falsy values the same, which is causing problems now that falsy values can come back and are valid values (see T293639: Ensure the number 0 is a valid value for formattedData). There are a few symptoms I've noted while working on IP Info:

  • If null is returned by a GeoIp2InfoRetriever method, it's unclear if it's because the reader wasn't found or the data wasn't found and it's not clear which
  • getProxyType returns false if it's actually false in the database but also if it's not found
  • non-specific treatment of falsy values gets bubbled to the renderer, where if ( propertyValue ) is evaluated to determine whether or not the API returned data

Some possible solutions:

  • be specific about what null and false mean and explicitly check propertyValue === null in IpInfoWidget#generatePropertyMarkup
  • throw an error if a reader isn't found*
  • don't return values if they don't exist**

*I don't know if we did this intentionally and if we did, why
**The Info type classes are all fairly rigid in what they expect to come back which is why you'll see failures return as null in some cases or [] in others. I don't know why they don't support "info does not exist" values like false

-Add to README.md

Event Timeline

ARamirez_WMF renamed this task from Be more specific about falsy values in IP Info api to Be more specific about falsy values in IP Info api [1H].Nov 17 2021, 5:39 PM
ARamirez_WMF moved this task from Triage/To be Estimated to IP Info on the Anti-Harassment board.
ARamirez_WMF moved this task from IP Info to Cards ready for development on the Anti-Harassment board.
ARamirez_WMF added a project: Spike.
Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptNov 17 2021, 5:41 PM
Tchanders updated the task description. (Show Details)

Change 753697 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/IPInfo@master] Normalize returned null values

https://gerrit.wikimedia.org/r/753697

Change 753697 merged by jenkins-bot:

[mediawiki/extensions/IPInfo@master] Normalize returned null values

https://gerrit.wikimedia.org/r/753697