hi @ekilah I really appreciate this very keen eye here.

Just from personal experience with Typescript, I would highly recommend never distinguishing between undefined and null in your code. For example at Plaid we always perform checks in the form of error != null, instead of error !== null. This is hard to enforce in a linter, but in my opinion, a good style guide should encourage this.

That said I am going to file a ticket internally for this. I suspect we would much rather just fix this clearly undesirable behavior of the SDK than add undefined as a possible error value here.

Does the metadata from the onExit give any hints as to what is happening? Do you have any request ids or session ids?

A screenshot of what we log to our error reporting service from one of the occurrences we had of this, before I fixed it, shows that the metadata was also null or undefined (unfortunately the nature of JSON.stringify is that the bug reporting service here merges both null and undefined to "null" which made figuring this out more difficult, funny enough):

We don't currently log the Plaid request IDs, and I've since fixed the error on our end, so I probably won't be able to produce those for you unfortunately.

That said I am going to file a ticket internally for this. I suspect we would much rather just fix this clearly undesirable behavior of the SDK than add undefined as a possible error value here.

Your call, though until/unless this is resolved, others might hit this issue, and adding | undefined is pretty cheap and easy to remove again later, so if it were my decision, I'd lean towards correcting the typing in the mean time. Depends on your release cycles and how long until this might be fixed too.

Anyway, thanks for the reply! Sorry I can't provide those IDs for you. If I can do anything else to help, let me know!

Re: TS thoughts (more of a for-fun response, not super important to the topic at hand)

I agree in general that a language having two nullish types like JS does is tricky, and is best navigated with caution. That's especially true when writing code against third-party libraries. However, using TypeScript is a great way to avoid errors with mishandling them, as long as typings are correctly describing the possibilities. If the typings from Plaid were correctly including | undefined, and we were using !== null, we would have gotten a compiler error telling us that we weren't handling undefined properly.

That's why I made the issue here - I disagree that != checks are better than !== checks at a high level (though obviously it'd be great for avoiding bugs like this one), but that's a bit more of a tabs-vs-spaces flame war that we can leave to other discussion boards 😛

note to self: wonder if this is related to #204 👀

hey @ekilah I chatted with the team yesterday. We definitely want to fix this and always return just null, but because we are more focused on some migrations we are doing in the longer term that will result in this more ideal behavior anyways, its unlikely we will fix this behavior in our soon-to-be-deprecated code, so i'll add the type you suggested in the meantime.

Thanks for the issue report!

@ekilah can you tell me if you have seen any instances of this in the last ~2 weeks in production environment? we finally rolled out the architectural change I was talking about above which should resolve the undefined vs null values in callbacks. They should always be null now afaik.

Hi @skylarmb - unfortunately, I can't tell you one way or another, since I fixed/prevented this on our side when I reported this. But, glad to hear it's fixed upstream! 🥳