vapor-community / postgresql

Robust PostgreSQL interface for Swift
MIT License
131 stars 33 forks source link

Preparations causing fatal error #43

Closed pruthvikar closed 7 years ago

pruthvikar commented 7 years ago

Expected Behavior

Preparations should run and vapor should report Database prepared on OSX and Linux

Current Behavior

Preparations run as expected on OSX but not on Linux.

Preparations cause fatal error: unexpectedly found nil while unwrapping an Optional value

Postgres also encounters the following ERROR: relation "fluent" does not exist at character 43 when executing this statement STATEMENT: SELECT COUNT(*) as _fluent_aggregate FROM "fluent"

Possible causes

Seems like the way preparation history is being tracked is causing an issue when trying to store in the db. Maybe the fluent table is not being set up properly?

Steps to Reproduce

  1. When running a simple Vapor application on this docker image based on the official swift image
  2. Postgres running on this image
  3. Compile and run a vapor project with a preparation

Details

Here is the full (yet limited) stack trace. It is cause after the first preparation has finished

Current stack trace:
0    libswiftCore.so                    0x00007f3ee02066b0 swift_reportError + 120
1    libswiftCore.so                    0x00007f3ee02210d0 _swift_stdlib_reportFatalError + 62
2    libswiftCore.so                    0x00007f3ee001acb6 <unavailable> + 1186998
3    libswiftCore.so                    0x00007f3ee017a39d <unavailable> + 2626461
4    libswiftCore.so                    0x00007f3ee001acb6 <unavailable> + 1186998
5    libswiftCore.so                    0x00007f3ee0135410 specialized _fatalErrorMessage(StaticString, StaticString, file : StaticString, line : UInt, flags : UInt32) -> Never + 96
6    Server                             0x00000000007d06fc <unavailable> + 3999484
7    Server                             0x00000000007d0419 <unavailable> + 3998745
8    libpthread.so.0                    0x00007f3edf1dda99 <unavailable> + 60057
9    libswiftCore.so                    0x00007f3ee0214e90 swift_once + 90
10   Server                             0x00000000007d085a <unavailable> + 3999834
11   Server                             0x00000000007d89e0 <unavailable> + 4032992
12   Server                             0x00000000007d8d2e <unavailable> + 4033838
13   Server                             0x00000000007dad00 <unavailable> + 4041984
14   Server                             0x00000000007c74dc <unavailable> + 3962076
15   Server                             0x00000000007c760a <unavailable> + 3962378
16   Server                             0x00000000007c7709 <unavailable> + 3962633
17   libswiftCore.so                    0x00007f3edfffcd10 Collection.map<A> ((A.Iterator.Element) throws -> A1) throws -> [A1] + 762
18   Server                             0x00000000007c7397 <unavailable> + 3961751
19   Server                             0x00000000007cb471 <unavailable> + 3978353
20   Server                             0x00000000007e31e5 <unavailable> + 4076005
21   Server                             0x00000000007e2584 <unavailable> + 4072836
22   Server                             0x00000000007e3eb5 <unavailable> + 4079285
23   Server                             0x000000000074abeb <unavailable> + 3451883
24   Server                             0x000000000074779a <unavailable> + 3438490
25   Server                             0x00000000007475e5 <unavailable> + 3438053
26   Server                             0x00000000007326d9 <unavailable> + 3352281
27   Server                             0x00000000007188cc <unavailable> + 3246284
28   Server                             0x000000000071abb5 <unavailable> + 3255221
29   Server                             0x00000000007193ba <unavailable> + 3249082
30   Server                             0x00000000006dd9ed <unavailable> + 3004909
31   Server                             0x00000000007367ab <unavailable> + 3368875
32   Server                             0x0000000000735d96 <unavailable> + 3366294
33   Server                             0x0000000000795910 <unavailable> + 3758352
34   Server                             0x00000000007910fa <unavailable> + 3739898
35   Server                             0x000000000079187b <unavailable> + 3741819
36   Server                             0x000000000061ffdc <unavailable> + 2228188
37   Server                             0x000000000062501d <unavailable> + 2248733
38   Server                             0x0000000000624e0a <unavailable> + 2248202
39   Server                             0x00000000009caab6 <unavailable> + 6073014
40   Server                             0x000000000042cd50 main + 38
41   libc.so.6                          0x00007f3eddad1740 __libc_start_main + 240
42   Server                             0x000000000042cc79 <unavailable> + 183417
Illegal instruction
pruthvikar commented 7 years ago

I've traced the error to this line which causes the crash

calendar.timeZone = TimeZone(abbreviation: "UTC")!

natebird commented 7 years ago

Maybe because it is force unwrapping the TimeZone. Can you edit the file on your linux system and see if that is the fix?

pruthvikar commented 7 years ago

I could only set it to a non optional so did this and it worked. calendar.timeZone = TimeZone(abbreviation: "UTC") ?? TimeZone.current

None of the TimeZone initializers from an identifier or abbreviation seem to work on the swift docker image. I can only get a nil from them.

natebird commented 7 years ago

Go ahead and create with that fix.

pruthvikar commented 7 years ago

Would this cause an issue when run on a different timezone? This is what it would become

static let referenceDate: Date = {
            let components = DateComponents(year: 2000, month: 1, day: 1)
            var calendar = Calendar(identifier: .gregorian)
            calendar.timeZone = TimeZone(abbreviation: "UTC") ?? TimeZone.current
            return calendar.date(from: components)!
        }()

Not sure what this reference date is used for exactly.

Alternatively what about something like this?

static let referenceDate = Date(timeIntervalSinceReferenceDate: 0)

pruthvikar commented 7 years ago

I'm actually not sure why the reference date exists. I can only find 2 references to it which seem to be better server by just using Date(timeIntervalSinceReferenceDate: anotherDate)

The current uses are date.timeIntervalSince(BinaryUtils.TimestampConstants.referenceDate) and Date(timeInterval: interval, since: TimestampConstants.referenceDate)

natebird commented 7 years ago

It think you are right. Submit a PR and @sroebert can review it. He knows this library better than I do. I want to make sure we aren't missing some edge case.

natebird commented 7 years ago

Something else is going on here. The tests aren't passing unless you set the TimeZone. I'm interested in why it fails on Linux but not macOS. I think there is some nuance between the two we are missing.

pruthvikar commented 7 years ago

Turns out the reference date tested against was a year before. Changed the code to match the tests. They should pass again now.

sroebert commented 7 years ago

I am wondering why this crashes on your side, but not in the unit tests run on Linux. Which Swift version are you using? I am assuming the latest.

I know there are some issues with TimeZone in Foundation on Linux, maybe it is because other function are called before that.

Either way, using a direct reference time is fine. I did it like this, to have a clear indication of when the reference time actually is. But with the comment it seems fine.

pruthvikar commented 7 years ago

On the official Docker image for Swift TimeZone(abbreviation: "UTC") was returning nil for me. Even though there were timezones available in TimeZone.abbreviationDictionary. I didn't get a chance to investigate further.

vzsg commented 7 years ago

I cannot test now, but perhaps the tzdata package is not installed in that image?

pruthvikar commented 7 years ago

@vzsg spot on, just tested after installing tzdata and it returns a TimeZone!

natebird commented 7 years ago

@pruthvikar, @vzsg - Do you think we should revert the PR?

sroebert commented 7 years ago

I actually like that with the new solution you do not have to have tzdata installed.

JustinM1 commented 7 years ago

not building on heroku either. any current workaround?

pruthvikar commented 7 years ago

@JustinM1 are you receiving the same error?

Installing tzdata worked for me.

pruthvikar commented 7 years ago

@natebird Since this PR reduces dependencies, I also prefer keeping it :)

natebird commented 7 years ago

I agree. 👍 Also, less reverts.

JustinM1 commented 7 years ago

@pruthvikar would you happen to know how to go about installing that with heroku?

vzsg commented 7 years ago

Installing a system package won't work on Heroku, so we really need a solution that 1) works and 2) doesn't have extra external dependencies.

sroebert commented 7 years ago

Ok, so should I make the 2.0.1 release?

vzsg commented 7 years ago

Yeah, please 👍