haraka / haraka-net-utils

haraka network utilities
https://www.npmjs.com/package/haraka-net-utils
MIT License
2 stars 13 forks source link

Fix ESERVFAIL crash on timeout NS #90

Closed analogic closed 3 months ago

coveralls commented 3 months ago

Pull Request Test Coverage Report for Build 9362569134

Details


Totals Coverage Status
Change from base Build 8931345490: 0.01%
Covered Lines: 650
Relevant Lines: 708

💛 - Coveralls
msimerson commented 3 months ago

This doesn't fix the issue. The two DNS errors that are "non-fatal" essentially mean, "try again but do an A query instead of MX", which will may resolve to an implicit MX. In the case of a SERVFAIL, that's almost certainly going to fail as well (which it does):

❯ drill kamenoprumysl.cz MX
;; ->>HEADER<<- opcode: QUERY, rcode: SERVFAIL, id: 60612
;; flags: qr rd ra ; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 0 
;; QUESTION SECTION:
;; kamenoprumysl.cz.    IN  MX

;; ANSWER SECTION:

;; AUTHORITY SECTION:

;; ADDITIONAL SECTION:

;; Query time: 1858 msec
;; WHEN: Tue Jun  4 09:14:51 2024
;; MSG SIZE  rcvd: 34

and then...

❯ drill kamenoprumysl.cz A
;; ->>HEADER<<- opcode: QUERY, rcode: SERVFAIL, id: 32247
;; flags: qr rd ra ; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 0 
;; QUESTION SECTION:
;; kamenoprumysl.cz.    IN  A

;; ANSWER SECTION:

;; AUTHORITY SECTION:

;; ADDITIONAL SECTION:

;; Query time: 4315 msec
;; WHEN: Tue Jun  4 09:14:58 2024
;; MSG SIZE  rcvd: 34
analogic commented 3 months ago

I don't really understand why, but it doesn't crash anymore, even a bounce with the message "Error: Nowhere to deliver mail to for domain: kamenoprumysl.cz" is generated :)

So there must be a place where err leaks and backtrace does not help. Hmm

msimerson commented 3 months ago

I don't really understand why

I expect it's because your DNS server has cached the MX lookup failure, so the subsequent A query fails faster.

So there must be a place where err leaks and backtrace does not help.

I'm assuming that you don't have any get_mx hook returning for kamenoprumysl.cz and so the entry point to get_mx is here. If the promise times out, .then won't get called but the .catch will and get_mx_error will run. Or it should.

analogic commented 3 months ago

Could be the reason that error handling should not use the class's functions, e.g. logging?

2024-06-04 20:06:43.655883500  [CRIT] [-] [core] TypeError: Cannot read properties of undefined (reading 'lognotice')
2024-06-04 20:06:43.655923500  [CRIT] [-] [core]     at get_mx_error (/usr/lib/node_modules/Haraka/outbound/hmail.js:246:14)
2024-06-04 20:06:43.656750500  [NOTICE] [-] [core] Shutting down

Sometimes I really don't like javascript, it makes me feel like I'm not intelligent enough.

analogic commented 3 months ago

closing in favour of https://github.com/haraka/Haraka/pull/3376

msimerson commented 3 months ago

Sometimes I really don't like javascript, it makes me feel like I'm not intelligent enough.

I know that feeling. But it's mostly that older/legacy JS has an abundance of rough edges. I find writing newer JS to be less onerous.